From a859e78f4b572abb6f58c85a153d655accb62f6f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:16:32 +0000 Subject: [PATCH 01/23] fix: marketplace update --save-edits misses cross-module associations into the module An association a local module owns that points at an entity of the marketplace module was never saved by --save-edits, and the update did not refuse over it because the module itself was unedited. The update then deleted it: DROP MODULE removes the module's entities and DeleteEntity cascades into every other domain model, removing cross-module associations that reach a deleted entity by name (mendixlabs/mxcli#1502). SnapshotModule now also captures those associations as Snapshot.Dependents (from the catalog's associations view). They are carried on the drift report, saved as association-_.mdl, counted by the update's local-edit gate, listed by `marketplace diff`, and named in the update summary. They are kept out of LocallyModified: they are not edits to the marketplace module. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FWsKVhYYsv4mgsgUwEGp3z --- ...dits-misses-cross-module-associations.json | 1 + cmd/mxcli/cmd_marketplace_update.go | 26 +++- .../cmd_marketplace_update_dependents_test.go | 52 +++++++ cmd/mxcli/marketplace/compare.go | 20 ++- cmd/mxcli/marketplace/dependent_assoc_test.go | 137 ++++++++++++++++++ cmd/mxcli/marketplace/report.go | 13 ++ cmd/mxcli/marketplace/snapshot.go | 53 +++++++ cmd/mxcli/marketplace/update.go | 41 +++++- ...ce-save-edits-cross-module-association.mdl | 33 +++++ 9 files changed, 373 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-14-marketplace-save-edits-misses-cross-module-associations.json create mode 100644 cmd/mxcli/cmd_marketplace_update_dependents_test.go create mode 100644 cmd/mxcli/marketplace/dependent_assoc_test.go create mode 100644 mdl-examples/bug-tests/1502-marketplace-save-edits-cross-module-association.mdl diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-14-marketplace-save-edits-misses-cross-module-associations.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-14-marketplace-save-edits-misses-cross-module-associations.json new file mode 100644 index 0000000000..751356799a --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-14-marketplace-save-edits-misses-cross-module-associations.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-14", "symptom": "`mxcli marketplace update --to --save-edits ` saves only elements owned by the module being replaced; an association a LOCAL module owns that points at an entity of the marketplace module (e.g. a local Config entity linked to the module's Settings entity) is not saved, the update proceeds without --force because the module itself is unedited, and afterwards the association is gone and Studio Pro reports the missing association (mendixlabs/mxcli#1502).", "cause": "Not merely a save gap: the update DESTROYS the association. `PerformUpdate` runs DROP MODULE, which deletes the module's entities, and `DeleteEntity` (mdl/backend/modelsdk/domainmodel_alter.go) cascades into every other domain model, removing cross-module associations whose TO end names the deleted entity. `SnapshotModule` enumerated only `objects WHERE ModuleName = `, so the comparison, the gate and --save-edits never saw an element stored in another module. Fixed by `Snapshot.Dependents` (catalog `associations` view: other module, FROM/TO prefixed `.`, matched with substr since `_` is a LIKE wildcard), carried as `Report.Dependents`, saved as `association-_.mdl`, counted by `gateOnLocalEdits`, listed in `marketplace diff` and the update summary.", "file": "cmd/mxcli/marketplace/snapshot.go", "insight": "The question to ask of any replace-a-module path is not 'what does the module contain' but 'what does deleting it cascade into' \u2014 read DeleteEntity's cascade before trusting a module-scoped enumeration. The cheapest proof is a two-line characterisation test (create the cross association, PerformUpdate, DESCRIBE it fails) on the vendored expr-checker fixture, which has MyFirstModule beside Administration; it doubles as the control that something was at risk. Keep dependents OUT of LocallyModified: they are not edits to the marketplace module, and reporting them as such would make `diff` accuse the module of a change the user made elsewhere \u2014 gate on them separately. The saved replay creates the association with a new GUID, so link data is not guaranteed to survive; preserving the association's raw BSON across the drop is the real fix and is not done here.", "refs": ["mendixlabs/mxcli#1502"]} diff --git a/cmd/mxcli/cmd_marketplace_update.go b/cmd/mxcli/cmd_marketplace_update.go index c96365cf5c..e902da3a0d 100644 --- a/cmd/mxcli/cmd_marketplace_update.go +++ b/cmd/mxcli/cmd_marketplace_update.go @@ -34,6 +34,11 @@ any. Use --save-edits to write them out as re-executable MDL first, and --force to proceed. Studio Pro discards them without asking; this at least tells you what they were. +Associations that OTHER modules own and that point into this module are removed +with it, because they reach entities the update deletes. They count as local work +here too: the update refuses over them, and --save-edits writes each one as +association-_.mdl to replay afterwards. + This does not roll back. Work on a copy or have the project in version control: if a step fails partway, the module has already been removed.`, Example: ` # What would this update touch, and have I edited any of it? @@ -117,6 +122,7 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { // impossible when that version has been unpublished. A blank 11.13 app ships // NanoflowCommons 6.0.0 and the 6.x line now starts at 6.1.1, so the module // that most needs updating is the one whose baseline cannot be built. + var drift *marketplace.Report if noBaseline { fmt.Fprintf(out, "\n--no-baseline: skipping the local-edit check for %s.\n", moduleName) fmt.Fprintln(out, " Any local edits to this module will be discarded without being named.") @@ -140,7 +146,7 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { if perr != nil { return fmt.Errorf("read %s from its published package: %w", basePkgModule, perr) } - drift := marketplace.Compare(installed, published) + drift = marketplace.Compare(installed, published) if saveEdits != "" { written, unsaved, serr := marketplace.SaveEdits(saveEdits, drift) @@ -176,6 +182,9 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { return fmt.Errorf("%w\n\nThe project may be mid-update — %s could be missing. Restore from version control", err, moduleName) } + if drift != nil { + res.DependentsRemoved = drift.DependentNames() + } reportUpdate(out, res) return nil } @@ -186,6 +195,9 @@ func runMarketplaceUpdate(cmd *cobra.Command, args []string) error { func gateOnLocalEdits(out io.Writer, drift *marketplace.Report, force bool, saveEdits string) error { result := marketplace.NewDiffResult("", "", "", drift) edited := append(append([]string{}, result.Modified...), result.OnlyInstalled...) + // Associations other modules own that reach into this one. Not edits to the + // module, but the update deletes them just the same (mendixlabs/mxcli#1502). + edited = append(edited, result.Dependents...) if len(edited) == 0 { if !result.Verified { @@ -202,6 +214,11 @@ func gateOnLocalEdits(out io.Writer, drift *marketplace.Report, force bool, save if !force { msg := fmt.Sprintf("refusing to update: %d element(s) have been changed locally and the update would discard them:\n", len(edited)) + if len(result.Dependents) > 0 { + msg = fmt.Sprintf("refusing to update: the update would discard %d local element(s) "+ + "(%d association(s) in other modules reach into %s and are removed with it):\n", + len(edited), len(result.Dependents), drift.Module) + } for _, e := range edited { msg += " " + e + "\n" } @@ -284,6 +301,13 @@ func reportUpdate(out io.Writer, r *marketplace.UpdateResult) { fmt.Fprintf(out, " %s\n", g) } } + if len(r.DependentsRemoved) > 0 { + fmt.Fprintf(out, "\n Removed from other modules (%d) — they reached into %s; replay them from --save-edits:\n", + len(r.DependentsRemoved), r.Module) + for _, d := range r.DependentsRemoved { + fmt.Fprintf(out, " %s\n", d) + } + } reportSecurityReconcile(out, r) // A newer module's pages reference widget definitions the project has not // resynced, so `mx check` reports CE0463 until it is told to. Measured on diff --git a/cmd/mxcli/cmd_marketplace_update_dependents_test.go b/cmd/mxcli/cmd_marketplace_update_dependents_test.go new file mode 100644 index 0000000000..0215059a45 --- /dev/null +++ b/cmd/mxcli/cmd_marketplace_update_dependents_test.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/marketplace" +) + +// An association another module owns, reaching into the module being replaced, +// is deleted by the update (mendixlabs/mxcli#1502). The module itself is +// unedited, so before the fix the gate let the update through without --force +// and without naming what it was about to remove. +func TestGateOnLocalEdits_RefusesWhenAnAssociationReachesIn(t *testing.T) { + drift := &marketplace.Report{ + Module: "Administration", + Findings: []marketplace.Finding{ + {Key: marketplace.ElementKey{Type: "ENTITY", Name: "Account"}, Verdict: marketplace.Unchanged}, + }, + Dependents: []marketplace.Element{{ + Key: marketplace.ElementKey{Type: "ASSOCIATION", Name: "MyFirstModule.Config_Account"}, + MDL: "create or modify association MyFirstModule.Config_Account\nfrom MyFirstModule.Config to Administration.Account;", + }}, + } + + var out bytes.Buffer + err := gateOnLocalEdits(&out, drift, false, "") + if err == nil { + t.Fatal("update allowed without --force although it removes MyFirstModule.Config_Account") + } + if !strings.Contains(err.Error(), "MyFirstModule.Config_Account") { + t.Errorf("refusal does not name the association: %v", err) + } + + out.Reset() + if err := gateOnLocalEdits(&out, drift, true, ""); err != nil { + t.Fatalf("--force must proceed: %v", err) + } + if !strings.Contains(out.String(), "MyFirstModule.Config_Account") { + t.Errorf("--force output does not name what is removed:\n%s", out.String()) + } + + // Control: the same report without the dependent passes the gate, so the + // refusal above is the dependent's doing and nothing else's. + drift.Dependents = nil + if err := gateOnLocalEdits(&out, drift, false, ""); err != nil { + t.Fatalf("control: an unedited module with no dependents must pass the gate: %v", err) + } +} diff --git a/cmd/mxcli/marketplace/compare.go b/cmd/mxcli/marketplace/compare.go index a1b68aba0a..9750990b8f 100644 --- a/cmd/mxcli/marketplace/compare.go +++ b/cmd/mxcli/marketplace/compare.go @@ -39,6 +39,21 @@ type Finding struct { type Report struct { Module string Findings []Finding + // Dependents are elements of OTHER modules that the update destroys: the + // cross-module associations that reach into this module by name. They are + // not edits to the module and are not counted by LocallyModified, but + // replacing the module deletes them all the same — see + // Snapshot.Dependents (mendixlabs/mxcli#1502). + Dependents []Element +} + +// DependentNames lists the dependents' keys, for reporting. +func (r *Report) DependentNames() []string { + out := make([]string, 0, len(r.Dependents)) + for _, d := range r.Dependents { + out = append(out, d.Key.String()) + } + return out } // Counts tallies the report by verdict. @@ -82,7 +97,10 @@ func (r *Report) Clean() bool { // only in the project is something the user added, while one only in the package // is something they deleted (or the version differs). func Compare(installed, pkg *Snapshot) *Report { - rep := &Report{Module: installed.Module} + // Only the installed side has dependents worth keeping: they are the + // project's own work. A reference project built from the package has no + // other module that could reach into this one. + rep := &Report{Module: installed.Module, Dependents: installed.Dependents} seen := make(map[ElementKey]bool, len(installed.Elements)+len(pkg.Elements)) keys := make([]ElementKey, 0, len(seen)) diff --git a/cmd/mxcli/marketplace/dependent_assoc_test.go b/cmd/mxcli/marketplace/dependent_assoc_test.go new file mode 100644 index 0000000000..b08075ef37 --- /dev/null +++ b/cmd/mxcli/marketplace/dependent_assoc_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: Apache-2.0 + +package marketplace + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/executor" +) + +// A local module that links to a marketplace module's entity owns that link: a +// cross-module association is stored in the FROM entity's module. It is still the +// update's casualty — DROP MODULE deletes the marketplace entity, and DeleteEntity +// cascades into every other domain model and removes the cross-module +// associations that reach the entity by name. `--save-edits` enumerated only the +// replaced module, so the one piece of local work the update was certain to +// destroy was the one it never saved (mendixlabs/mxcli#1502). +const crossModuleAssocMDL = ` +create persistent entity MyFirstModule.Config ( Note: String(100) ); +create association MyFirstModule.Config_Account from MyFirstModule.Config to Administration.Account; +` + +const crossModuleAssoc = "MyFirstModule.Config_Account" + +// describeAssoc returns the DESCRIBE output for an association, and whether it exists. +func describeAssoc(t *testing.T, mprPath, qualified string) (string, bool) { + t.Helper() + var sink bytes.Buffer + exec := executor.New(&sink) + exec.SetBackendFactory(testBackend) + defer exec.Close() + if err := exec.Execute(&ast.ConnectStmt{Path: mprPath}); err != nil { + t.Fatalf("connect: %v", err) + } + qn, err := splitQualified(qualified) + if err != nil { + t.Fatal(err) + } + if err := exec.Execute(&ast.DescribeStmt{ObjectType: ast.DescribeAssociation, Name: qn}); err != nil { + return err.Error(), false + } + return sink.String(), true +} + +// The mechanism, and the control for the test below: an update of Administration +// removes a local module's association into it. Without this the save test could +// pass against a fixture where nothing was ever at risk. +func TestPerformUpdate_RemovesCrossModuleAssociationsIntoTheModule(t *testing.T) { + target := copyFixture(t) + execMDL(t, target, crossModuleAssocMDL) + if _, ok := describeAssoc(t, target, crossModuleAssoc); !ok { + t.Fatalf("control: %s was not created", crossModuleAssoc) + } + + if _, err := PerformUpdate(target, copyFixture(t), emptyMpk(t), "Administration", + "4.3.2", "4.3.3", "00000000-0000-0000-0000-000000000000", testBackend); err != nil { + t.Fatalf("PerformUpdate: %v", err) + } + if out, ok := describeAssoc(t, target, crossModuleAssoc); ok { + t.Fatalf("expected the update to remove %s (it reaches a dropped entity); it survived:\n%s\n"+ + "if the update now preserves it, the save below is no longer the only rescue — revisit #1502", + crossModuleAssoc, out) + } +} + +// The reported symptom: --save-edits saves nothing for the association, so after +// the update it is gone with no file to replay. +func TestSaveEdits_SavesCrossModuleAssociationsIntoTheModule(t *testing.T) { + target := copyFixture(t) + execMDL(t, target, crossModuleAssocMDL) + + installed, err := SnapshotModule(target, "Administration", testBackend) + if err != nil { + t.Fatalf("snapshot installed: %v", err) + } + published, err := SnapshotModule(copyFixture(t), "Administration", testBackend) + if err != nil { + t.Fatalf("snapshot package: %v", err) + } + drift := Compare(installed, published) + + if got := drift.DependentNames(); len(got) != 1 || got[0] != "ASSOCIATION "+crossModuleAssoc { + t.Fatalf("dependents = %v; want exactly ASSOCIATION %s", got, crossModuleAssoc) + } + // The module itself is untouched: the association lives in MyFirstModule. + // It must not be passed off as an edit to Administration. + if drift.LocallyModified() { + t.Errorf("Administration is unedited; the association belongs to MyFirstModule") + } + + // `marketplace diff` reports them too, without counting them as edits. + if res := NewDiffResult("Administration", "", "", drift); len(res.Dependents) != 1 || res.LocallyModified { + t.Errorf("diff result: dependents %v, locallyModified %v", res.Dependents, res.LocallyModified) + } + + dir := t.TempDir() + written, unsaved, err := SaveEdits(dir, drift) + if err != nil { + t.Fatalf("SaveEdits: %v", err) + } + if len(unsaved) != 0 { + t.Errorf("unsaved = %v", unsaved) + } + want := filepath.Join(dir, "association-MyFirstModule_Config_Account.mdl") + if len(written) != 1 || written[0] != want { + t.Fatalf("written = %v; want [%s] — this is mendixlabs/mxcli#1502", written, want) + } + body, err := os.ReadFile(want) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "association MyFirstModule.Config_Account") { + t.Fatalf("saved file does not recreate the association:\n%s", body) + } + + // The file is only a rescue if it replays: update, then exec it. + if _, err := PerformUpdate(target, copyFixture(t), emptyMpk(t), "Administration", + "4.3.2", "4.3.3", "00000000-0000-0000-0000-000000000000", testBackend); err != nil { + t.Fatalf("PerformUpdate: %v", err) + } + if _, ok := describeAssoc(t, target, crossModuleAssoc); ok { + t.Fatal("control: the update was expected to remove the association") + } + execMDL(t, target, string(body)) + out, ok := describeAssoc(t, target, crossModuleAssoc) + if !ok { + t.Fatalf("replaying %s did not restore the association: %s", want, out) + } + if !strings.Contains(out, "Administration.Account") { + t.Errorf("restored association does not reach Administration.Account:\n%s", out) + } +} diff --git a/cmd/mxcli/marketplace/report.go b/cmd/mxcli/marketplace/report.go index f1ff563931..0d895a8e5f 100644 --- a/cmd/mxcli/marketplace/report.go +++ b/cmd/mxcli/marketplace/report.go @@ -32,6 +32,10 @@ type DiffResult struct { OnlyPackage []string `json:"onlyPackage,omitempty"` Unknown []UnreadableElement `json:"unknown,omitempty"` UnchangedCount int `json:"unchangedCount"` + // Dependents are associations other modules own that reach into this one. + // Not local edits to the module — LocallyModified ignores them — but an + // update removes them (mendixlabs/mxcli#1502). + Dependents []string `json:"dependents,omitempty"` // Upgrade is populated when --to was given: what changes between the // installed version and the target, and which of those the user has edited. @@ -79,6 +83,7 @@ func NewDiffResult(module, installedVersion, mendixVersion string, rep *Report) out.Unknown = append(out.Unknown, UnreadableElement{Element: f.Key.String(), Reason: f.Reason}) } } + out.Dependents = rep.DependentNames() sort.Strings(out.Modified) sort.Strings(out.OnlyInstalled) sort.Strings(out.OnlyPackage) @@ -135,6 +140,14 @@ func (d *DiffResult) WriteText(w io.Writer) error { } } + if len(d.Dependents) > 0 { + fmt.Fprintf(w, "\n Reaching in from other modules (%d) — an update removes these; --save-edits keeps them:\n", + len(d.Dependents)) + for _, e := range d.Dependents { + fmt.Fprintf(w, " dependent %s\n", e) + } + } + if d.Upgrade != nil { fmt.Fprintf(w, "\n Upgrading to %s would touch %d element(s)", d.TargetVersion, len(d.Upgrade.Touched)) if len(d.Upgrade.Conflicts) == 0 { diff --git a/cmd/mxcli/marketplace/snapshot.go b/cmd/mxcli/marketplace/snapshot.go index 21d665294a..bc24ae207d 100644 --- a/cmd/mxcli/marketplace/snapshot.go +++ b/cmd/mxcli/marketplace/snapshot.go @@ -125,6 +125,18 @@ func emptyBody(mdl string) bool { type Snapshot struct { Module string Elements map[ElementKey]Element + // Dependents are the cross-module associations OTHER modules own that reach + // into this one, keyed by qualified name (they are not this module's, so the + // unqualified-name convention of Elements does not apply). + // + // They are captured because replacing the module destroys them: DROP MODULE + // deletes this module's entities, and DeleteEntity cascades into every other + // domain model to remove the cross-module associations that reach a deleted + // entity by name. The association belongs to the user's module, so it is + // nobody's "edit" to this one — which is how --save-edits came to save every + // change the update would discard except the one it was certain to + // (mendixlabs/mxcli#1502). + Dependents []Element } // Keys returns the snapshot's element keys in a stable order. @@ -193,6 +205,14 @@ func SnapshotModule(mprPath, moduleName string, newBackend func() backend.FullBa } snap.Elements[key] = describeElement(exec, &sink, row) } + + deps, err := queryDependentAssociations(exec, moduleName) + if err != nil { + return nil, err + } + for _, row := range deps { + snap.Dependents = append(snap.Dependents, describeElement(exec, &sink, row)) + } return snap, nil } @@ -230,6 +250,39 @@ func queryModuleObjects(exec *executor.Executor, moduleName string) ([]moduleObj return out, nil } +// queryDependentAssociations finds the associations owned by other modules that +// have an end in moduleName. In practice that is the TO end — a cross-module +// association is stored with its FROM entity — but both ends are matched so a +// shape the catalog reports differently is not silently missed. +// +// The prefix is compared with substr rather than LIKE: module names may contain +// `_`, which LIKE reads as a wildcard. +func queryDependentAssociations(exec *executor.Executor, moduleName string) ([]moduleObject, error) { + cat := exec.Catalog() + if cat == nil { + return nil, fmt.Errorf("no catalog available for %s", moduleName) + } + quoted := "'" + strings.ReplaceAll(moduleName, "'", "''") + ".'" + n := fmt.Sprint(len(moduleName) + 1) + q := "SELECT QualifiedName FROM associations WHERE ModuleName <> '" + + strings.ReplaceAll(moduleName, "'", "''") + "' AND (" + + "substr(ToEntity, 1, " + n + ") = " + quoted + " OR substr(FromEntity, 1, " + n + ") = " + quoted + + ") ORDER BY QualifiedName" + res, err := cat.Query(q) + if err != nil { + return nil, fmt.Errorf("find associations into module %s: %w", moduleName, err) + } + out := make([]moduleObject, 0, len(res.Rows)) + for _, row := range res.Rows { + if len(row) < 1 { + continue + } + qn := fmt.Sprintf("%v", row[0]) + out = append(out, moduleObject{objectType: "ASSOCIATION", name: qn, qualifiedName: qn}) + } + return out, nil +} + // describeElement runs one DESCRIBE and captures its output. func describeElement(exec *executor.Executor, sink *bytes.Buffer, obj moduleObject) Element { key := ElementKey{Type: obj.objectType, Name: obj.name} diff --git a/cmd/mxcli/marketplace/update.go b/cmd/mxcli/marketplace/update.go index 5425b349e2..d2ab073c41 100644 --- a/cmd/mxcli/marketplace/update.go +++ b/cmd/mxcli/marketplace/update.go @@ -56,7 +56,18 @@ func SaveEdits(dir string, rep *Report) (written []string, unsaved []string, err unsaved = append(unsaved, fmt.Sprintf("%s (%s)", f.Key, f.Reason)) } } - if len(wanted) == 0 { + // Associations other modules own that reach into this one. They are not + // edits to the module, but the update deletes them (Snapshot.Dependents), + // so they are saved by the same rules as an edit. + var dependents []Element + for _, d := range rep.Dependents { + if ok, why := d.Conclusive(); !ok { + unsaved = append(unsaved, fmt.Sprintf("%s (%s)", d.Key, why)) + continue + } + dependents = append(dependents, d) + } + if len(wanted) == 0 && len(dependents) == 0 { return nil, unsaved, nil } if err := os.MkdirAll(dir, 0o755); err != nil { @@ -79,6 +90,29 @@ func SaveEdits(dir string, rep *Report) (written []string, unsaved []string, err } written = append(written, name) } + + for _, d := range dependents { + body := replayable(d.MDL) + if strings.TrimSpace(body) == "" { + unsaved = append(unsaved, d.Key.String()+" (no re-executable definition)") + continue + } + // The key is qualified (Module.Name); "association-Module_Name.mdl" + // keeps the owning module in the file name without a second dot. + name := filepath.Join(dir, safeFileName(ElementKey{ + Type: d.Key.Type, Name: strings.ReplaceAll(d.Key.Name, ".", "_")})+".mdl") + header := fmt.Sprintf("-- %s, as it stands in your project before the update.\n"+ + "-- It belongs to another module, but reaches into %s, and replacing %s\n"+ + "-- removes it along with the entity it points at.\n"+ + "-- Replay with: mxcli exec %s -p \n"+ + "-- Note: replaying creates it anew, with a new GUID. The runtime keys stored\n"+ + "-- data on an element's GUID, so do not count on its existing links surviving.\n\n", + d.Key, rep.Module, rep.Module, name) + if err := os.WriteFile(name, []byte(header+body+"\n"), 0o644); err != nil { + return written, unsaved, fmt.Errorf("write %s: %w", name, err) + } + written = append(written, name) + } return written, unsaved, nil } @@ -126,6 +160,11 @@ type UpdateResult struct { // "the package wanted a different version" is the fact that was missing. FilesSkipped []SkippedFile ForcedOverEdits []string + // DependentsRemoved names the associations other modules owned that reached + // into this one, which the update deleted along with the entities they + // pointed at (mendixlabs/mxcli#1502). Filled by the caller from the drift + // report; empty when no baseline was compared. + DependentsRemoved []string } // SkippedFile is one bundled file InstallPackageFiles chose not to write. diff --git a/mdl-examples/bug-tests/1502-marketplace-save-edits-cross-module-association.mdl b/mdl-examples/bug-tests/1502-marketplace-save-edits-cross-module-association.mdl new file mode 100644 index 0000000000..3bcb6ef29d --- /dev/null +++ b/mdl-examples/bug-tests/1502-marketplace-save-edits-cross-module-association.mdl @@ -0,0 +1,33 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1502: `marketplace update --save-edits` misses cross-module +-- associations that target entities inside the updated module +-- ============================================================================ +-- +-- Symptom (before fix): an association a LOCAL module owns, pointing at an +-- entity of the marketplace module, was never saved by --save-edits, and the +-- update (which did not refuse — the module itself was unedited) deleted it: +-- DROP MODULE removes the module's entities, and DeleteEntity cascades into +-- every other domain model to remove the cross-module associations that reach +-- a deleted entity by name. Studio Pro then reports the missing association. +-- +-- After fix: `marketplace diff` lists it under "Reaching in from other +-- modules", `marketplace update` refuses without --force and names it, and +-- --save-edits writes association-MyFirstModule_Config_Account.mdl, which +-- replays after the update. +-- +-- Usage (on a project with Administration installed from the marketplace): +-- mxcli exec mdl-examples/bug-tests/1502-marketplace-save-edits-cross-module-association.mdl -p app.mpr +-- mxcli marketplace diff 23513 -p app.mpr +-- mxcli marketplace update 23513 -p app.mpr --to --save-edits ./local-edits +-- mxcli marketplace update 23513 -p app.mpr --to --force +-- mxcli exec ./local-edits/association-MyFirstModule_Config_Account.mdl -p app.mpr +-- ============================================================================ + +create persistent entity MyFirstModule.Config ( + Note: String(100) +); + +create association MyFirstModule.Config_Account + from MyFirstModule.Config + to Administration.Account; From d798d83a41eadff8f46bfc1f420b5b53f1c67f25 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:43:56 +0000 Subject: [PATCH 02/23] fix(pages): describe and address legacy DataGrid columns and search fields (mendixlabs/mxcli#1459) findInWidgetChildren walked a Forms$DataGrid's ControlBar.Items but not its Columns or DataSource.SearchBar.Items, although both carry a stored Name, and describe printed the grid as one opaque comment line. The mutator now walks both lists (gated on $Type), and describe prints the grid's data source, buttons, columns and search fields by stored name, every line a comment. Tests: pagemutator drop/set Caption on a column and a search field (siblings byte-identical); executor describe test. Stubbing either fix fails the tests with `widget "column14" not found` / the opaque line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...legacy-datagrid-columns-unaddressable.json | 1 + .claude/skills/mendix/alter-page/SKILL.md | 9 + .../pagemutator/legacy_datagrid_1459_test.go | 199 ++++++++++++++++++ mdl/backend/pagemutator/legacygrid.go | 39 ++++ mdl/backend/pagemutator/mutator.go | 9 + mdl/executor/cmd_pages_describe.go | 7 +- .../cmd_pages_describe_legacy_grid.go | 130 ++++++++++++ mdl/executor/cmd_pages_describe_output.go | 3 + mdl/executor/cmd_pages_describe_parse.go | 9 + .../describe_legacy_datagrid_1459_test.go | 114 ++++++++++ 10 files changed, 518 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-backend/2026-10-13-legacy-datagrid-columns-unaddressable.json create mode 100644 mdl/backend/pagemutator/legacy_datagrid_1459_test.go create mode 100644 mdl/backend/pagemutator/legacygrid.go create mode 100644 mdl/executor/cmd_pages_describe_legacy_grid.go create mode 100644 mdl/executor/describe_legacy_datagrid_1459_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-13-legacy-datagrid-columns-unaddressable.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-13-legacy-datagrid-columns-unaddressable.json new file mode 100644 index 0000000000..e86e853843 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-13-legacy-datagrid-columns-unaddressable.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-13", "symptom": "describe printed a legacy Forms$DataGrid as one `-- Forms$DataGrid (grid1) -- NOT re-executable` line, so its columns and search fields had no names in the output; `alter page … drop column14` passed check -p --references and failed at exec with `widget \"column14\" not found`", "cause": "findInWidgetChildren walked the grid's ControlBar.Items but never its Columns (Forms$GridColumn) or DataSource.SearchBar.Items (Forms$*SearchField), although both carry a stored Name; parseRawWidget had no Forms$DataGrid case so describe never read them either", "file": "`mdl/backend/pagemutator/legacygrid.go` (`legacyGridItemLists`, used by `findInWidgetChildren` and `collectWidgetScopeInChildren`), `mdl/executor/cmd_pages_describe_legacy_grid.go`", "insight": "Unlike DataGrid2 columns (derived names, #78), legacy grid columns and search fields are NAMED — the gap was only which lists the finder walks, so the existing drop/set-Caption paths work unchanged once they are reachable. An unauthorable widget can still be printed structurally if every line stays a comment: exec of the output then drops the grid (as before) instead of building something different. Gate the walk on $Type: a LayoutGrid row also has a `Columns` key. No fixture holds a legacy grid (pedapp/testapp grep for Forms$DataGrid is empty), so tests build Studio Pro-shaped BSON from the property names in modelsdk/gen/pages", "refs": ["mendixlabs/mxcli#1459"]} diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index 3bb7c8047b..b76c6afa97 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -401,6 +401,15 @@ drop dgProducts column(Name)@2 The older dotted form `gridName.columnName` still works; it matches a name mxcli derives (the short attribute name, else the sanitized caption, else `colN`). +A **legacy (Dojo) data grid** (`Forms$DataGrid`, from an upgraded app) cannot be created in MDL, so `describe page` prints it as comments — but it lists every column, search field and control-bar button by its *stored* name. Those are addressed by that bare name, like any widget: + +```sql +drop column14, searchField3 -- one column and its search field +set (caption: 'Note') on column2 +``` + +Only that item is removed or changed; the rest of the grid is left as stored. To restructure the grid itself, replace it with a DataGrid 2. + ### ADD Variables - Add a Page Variable ```sql diff --git a/mdl/backend/pagemutator/legacy_datagrid_1459_test.go b/mdl/backend/pagemutator/legacy_datagrid_1459_test.go new file mode 100644 index 0000000000..fe0abd8cb7 --- /dev/null +++ b/mdl/backend/pagemutator/legacy_datagrid_1459_test.go @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "bytes" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1459: a legacy Forms$DataGrid's columns and search fields +// carry a stored Name, but the widget finder only searched the control bar, so +// `alter page … drop column14` passed check and failed at exec with +// `widget "column14" not found`. + +func legacyGridText(s string) bson.D { + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Texts$Text"}, + {Key: "Items", Value: bson.A{int32(3), bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Texts$Translation"}, + {Key: "LanguageCode", Value: "en_US"}, + {Key: "Text", Value: s}, + }}}, + } +} + +func legacyGridAttrRef(qn string) bson.D { + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: qn}, + {Key: "EntityRef", Value: nil}, + } +} + +func legacyGridColumn(name, caption, attr string) bson.D { + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$GridColumn"}, + {Key: "Name", Value: name}, + {Key: "Caption", Value: legacyGridText(caption)}, + {Key: "AttributeRef", Value: legacyGridAttrRef(attr)}, + {Key: "Editable", Value: false}, + {Key: "Width", Value: int32(50)}, + } +} + +func legacyGridSearchField(typ, name, caption, attr string) bson.D { + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: typ}, + {Key: "Name", Value: name}, + {Key: "Caption", Value: legacyGridText(caption)}, + {Key: "Type", Value: "Normal"}, + {Key: "AttributeRef", Value: legacyGridAttrRef(attr)}, + } +} + +func makeLegacyGridPage() bson.D { + return makeRawPage(bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$DataGrid"}, + {Key: "Name", Value: "grid1"}, + {Key: "DataSource", Value: bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$GridXPathSource"}, + {Key: "SearchBar", Value: bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$SearchBar"}, + {Key: "Items", Value: bson.A{int32(2), + legacyGridSearchField("Forms$ComparisonSearchField", "searchField1", "Number", "Shop.Order.Number"), + legacyGridSearchField("Forms$DropDownSearchField", "searchField2", "Note", "Shop.Order.Note"), + }}, + }}, + }}, + {Key: "ControlBar", Value: bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$GridControlBar"}, + {Key: "Items", Value: bson.A{int32(2), bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Forms$GridNewButton"}, + {Key: "Name", Value: "newButton1"}, + {Key: "Caption", Value: legacyGridText("New")}, + }}}, + }}, + {Key: "Columns", Value: bson.A{int32(2), + legacyGridColumn("column1", "Order number", "Shop.Order.Number"), + legacyGridColumn("column14", "Internal note", "Shop.Order.Note"), + legacyGridColumn("column2", "Date", "Shop.Order.Date"), + }}, + }) +} + +func legacyGridDoc(t *testing.T, raw bson.D) bson.D { + t.Helper() + r := findBsonWidget(raw, "grid1") + if r == nil { + t.Fatal("grid1 no longer resolves") + } + return r.widget +} + +func legacyGridSearchItems(t *testing.T, raw bson.D) bson.A { + t.Helper() + ds := bsonnav.DGetDoc(legacyGridDoc(t, raw), "DataSource") + return bsonnav.ToBsonA(bsonnav.DGet(bsonnav.DGetDoc(ds, "SearchBar"), "Items")) +} + +func marshalDoc(t *testing.T, v any) []byte { + t.Helper() + b, err := bson.Marshal(v) + if err != nil { + t.Fatal(err) + } + return b +} + +// itemNames lists the Name of every document in a stored list, and fails when +// the list lost its leading type marker. +func itemNames(t *testing.T, arr bson.A) []string { + t.Helper() + if len(arr) == 0 || !isListMarker(arr[0]) { + t.Fatalf("list lost its marker: %v", arr) + } + var out []string + for _, e := range arr[1:] { + out = append(out, bsonnav.DGetString(e.(bson.D), "Name")) + } + return out +} + +func TestDropLegacyGridColumnRemovesOnlyThatColumn(t *testing.T) { + raw := makeLegacyGridPage() + before := bsonnav.ToBsonA(bsonnav.DGet(legacyGridDoc(t, raw), "Columns")) + col1, col2 := marshalDoc(t, before[1]), marshalDoc(t, before[3]) + + m := New(raw, model.ID("page-1"), &stubWidgetDeps{}) + if err := m.DropWidget([]backend.WidgetRef{{Widget: "column14"}}); err != nil { + t.Fatalf("drop column14: %v", err) + } + after := bsonnav.ToBsonA(bsonnav.DGet(legacyGridDoc(t, m.rawData), "Columns")) + if got := itemNames(t, after); len(got) != 2 || got[0] != "column1" || got[1] != "column2" { + t.Fatalf("columns after drop = %v, want [column1 column2]", got) + } + if !bytes.Equal(marshalDoc(t, after[1]), col1) || !bytes.Equal(marshalDoc(t, after[2]), col2) { + t.Error("a sibling column changed: it must pass through as stored") + } + if got := itemNames(t, legacyGridSearchItems(t, m.rawData)); len(got) != 2 { + t.Errorf("search fields changed by a column drop: %v", got) + } +} + +func TestDropLegacyGridSearchFieldRemovesOnlyThatField(t *testing.T) { + raw := makeLegacyGridPage() + keep := marshalDoc(t, legacyGridSearchItems(t, raw)[1]) + + m := New(raw, model.ID("page-1"), &stubWidgetDeps{}) + if err := m.DropWidget([]backend.WidgetRef{{Widget: "searchField2"}}); err != nil { + t.Fatalf("drop searchField2: %v", err) + } + items := legacyGridSearchItems(t, m.rawData) + if got := itemNames(t, items); len(got) != 1 || got[0] != "searchField1" { + t.Fatalf("search fields after drop = %v, want [searchField1]", got) + } + if !bytes.Equal(marshalDoc(t, items[1]), keep) { + t.Error("the remaining search field changed") + } + if got := itemNames(t, bsonnav.ToBsonA(bsonnav.DGet(legacyGridDoc(t, m.rawData), "Columns"))); len(got) != 3 { + t.Errorf("columns changed by a search-field drop: %v", got) + } +} + +func TestSetCaptionOnLegacyGridColumnAndSearchField(t *testing.T) { + m := New(makeLegacyGridPage(), model.ID("page-1"), &stubWidgetDeps{}) + for name, caption := range map[string]string{"column14": "Note", "searchField1": "Order no."} { + if err := m.SetWidgetProperty(name, "Caption", caption); err != nil { + t.Fatalf("set Caption on %s: %v", name, err) + } + r := findBsonWidget(m.rawData, name) + if r == nil { + t.Fatalf("%s no longer resolves", name) + } + tr := bsonnav.ToBsonA(bsonnav.DGet(bsonnav.DGetDoc(r.widget, "Caption"), "Items")) + if got := bsonnav.DGetString(tr[1].(bson.D), "Text"); got != caption { + t.Errorf("%s caption = %q, want %q", name, got, caption) + } + } + if !m.FindWidget("column2") || m.WidgetScope()["searchField2"] == "" { + t.Error("legacy grid columns and search fields are not in the widget scope") + } +} diff --git a/mdl/backend/pagemutator/legacygrid.go b/mdl/backend/pagemutator/legacygrid.go new file mode 100644 index 0000000000..2e7a19124d --- /dev/null +++ b/mdl/backend/pagemutator/legacygrid.go @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" +) + +// namedItemList is a stored list of named elements: the document holding it +// and the key it is under, the pair findInWidgetArray takes. +type namedItemList struct { + doc bson.D + key string +} + +// legacyGridItemLists returns the named-item lists of a legacy (Dojo) +// Forms$DataGrid other than its control bar, which the finder already walks: +// the grid's Columns (Forms$GridColumn) and its source's SearchBar.Items +// (Forms$*SearchField). Each item carries a stored Name, so `drop ` and +// `set (Caption: …) on ` address them like any widget. Without this both +// were unreachable and the only remedy was replacing the grid +// (mendixlabs/mxcli#1459). +// +// Gated on the $Type: a LayoutGrid row also has a `Columns` key, holding +// layout columns rather than named grid columns. +func legacyGridItemLists(wDoc bson.D) []namedItemList { + if bsonnav.DGetString(wDoc, "$Type") != "Forms$DataGrid" { + return nil + } + lists := []namedItemList{{wDoc, "Columns"}} + if ds := bsonnav.DGetDoc(wDoc, "DataSource"); ds != nil { + if sb := bsonnav.DGetDoc(ds, "SearchBar"); sb != nil { + lists = append(lists, namedItemList{sb, "Items"}) + } + } + return lists +} diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 20dfaca8fc..4f5821d1d2 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -1535,6 +1535,12 @@ func findInWidgetChildren(wDoc bson.D, widgetName string) *bsonWidgetResult { return result } } + // Legacy DataGrid: Columns and the source's search bar (#1459). + for _, l := range legacyGridItemLists(wDoc) { + if result := findInWidgetArray(l.doc, l.key, widgetName); result != nil { + return result + } + } // CustomWidget (pluggable): Object.Properties[].Value.Widgets[] if strings.Contains(typeName, "CustomWidget") { @@ -2453,6 +2459,9 @@ func collectWidgetScopeInChildren(wDoc bson.D, scope map[string]model.ID) { if controlBar := bsonnav.DGetDoc(wDoc, "ControlBar"); controlBar != nil { collectWidgetScope(controlBar, "Items", scope) } + for _, l := range legacyGridItemLists(wDoc) { + collectWidgetScope(l.doc, l.key, scope) + } if strings.Contains(typeName, "CustomWidget") { if obj := bsonnav.DGetDoc(wDoc, "Object"); obj != nil { props := bsonnav.DGetArrayElements(bsonnav.DGet(obj, "Properties")) diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index f39888d840..8df653592e 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -753,8 +753,11 @@ type rawWidget struct { Children []rawWidget FilterWidgets []rawWidget // For Gallery filter widgets ControlBar []rawWidget // For DataGrid2 CONTROLBAR widgets - Rows []rawWidgetRow - DataSource *rawDataSource + // LegacyGrid holds a legacy Forms$DataGrid's named columns, search fields + // and control-bar buttons, printed as comments (mendixlabs/mxcli#1459). + LegacyGrid *rawLegacyGrid + Rows []rawWidgetRow + DataSource *rawDataSource // NamedDataSources holds a pluggable widget's datasources keyed by the // schema property each was stored under, and is set INSTEAD of DataSource // when the widget has more than one configured. Collapsing them to the one diff --git a/mdl/executor/cmd_pages_describe_legacy_grid.go b/mdl/executor/cmd_pages_describe_legacy_grid.go new file mode 100644 index 0000000000..9e028f70a0 --- /dev/null +++ b/mdl/executor/cmd_pages_describe_legacy_grid.go @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" +) + +// A legacy (Dojo) Forms$DataGrid. MDL cannot author one (MDL-WIDGET38), so +// describe still prints it as comments — but structurally, with the stored name +// of every column, search field and control-bar button, because those names are +// what `alter page … drop ` and `set (Caption: …) on ` address on +// the stored page. A single opaque line left them unaddressable and made the +// only remedy replacing the whole grid (mendixlabs/mxcli#1459). + +// rawLegacyGrid is what describe reads from a Forms$DataGrid. +type rawLegacyGrid struct { + ControlBar []rawLegacyGridItem + Columns []rawLegacyGridItem + SearchFields []rawLegacyGridItem +} + +// rawLegacyGridItem is one named element of a legacy grid: a control-bar +// button, a column or a search field. Kind is the $Type without its +// `Forms$` prefix; Type is a search field's visibility (Normal/Hidden/ReadOnly). +type rawLegacyGridItem struct { + Kind string + Name string + Caption string + Attribute string + Type string +} + +// parseLegacyGrid reads the named parts of a Forms$DataGrid: ControlBar.Items, +// Columns and the source's SearchBar.Items. +func parseLegacyGrid(ctx *ExecContext, w map[string]any) *rawLegacyGrid { + g := &rawLegacyGrid{} + if cb, ok := w["ControlBar"].(map[string]any); ok { + g.ControlBar = parseLegacyGridItems(ctx, cb["Items"]) + } + g.Columns = parseLegacyGridItems(ctx, w["Columns"]) + if ds, ok := w["DataSource"].(map[string]any); ok { + if sb, ok := ds["SearchBar"].(map[string]any); ok { + g.SearchFields = parseLegacyGridItems(ctx, sb["Items"]) + } + } + return g +} + +func parseLegacyGridItems(ctx *ExecContext, v any) []rawLegacyGridItem { + var out []rawLegacyGridItem + for _, e := range getBsonArrayElements(v) { + m, ok := e.(map[string]any) + if !ok { + continue + } + item := rawLegacyGridItem{ + Kind: strings.TrimPrefix(extractString(m["$Type"]), "Forms$"), + Name: extractString(m["Name"]), + Caption: extractTextCaption(ctx, m), + Type: extractString(m["Type"]), + } + if ref, ok := m["AttributeRef"].(map[string]any); ok && ref != nil { + item.Attribute = columnAttributeFromRef(ref) + } + if item.Attribute == "" { + // Older models store the binding as a path string instead. + item.Attribute = shortAttributeName(extractString(m["AttributePath"])) + } + out = append(out, item) + } + return out +} + +// outputLegacyGridV3 prints a legacy data grid. Every line is a comment: the +// widget cannot be built, so executing the describe output must not create +// anything in its place — it drops the grid, which the header line says. +func outputLegacyGridV3(ctx *ExecContext, prefix string, w rawWidget) { + describeDroppedWidget(ctx, w) + fmt.Fprintf(ctx.Output, "%s-- %s", prefix, w.Type) + if w.Name != "" { + fmt.Fprintf(ctx.Output, " (%s)", w.Name) + } + fmt.Fprint(ctx.Output, " -- NOT re-executable: mxcli cannot author this widget, so re-running this script would drop it\n") + g := w.LegacyGrid + if g == nil { + return + } + fmt.Fprintf(ctx.Output, "%s-- its columns, search fields and buttons are addressable by name: alter page … drop | set (Caption: '…') on \n", prefix) + if expr := dataSourceExpr(ctx, w.DataSource); expr != "" { + fmt.Fprintf(ctx.Output, "%s-- DataSource: %s\n", prefix, legacyCommentText(expr)) + } + for _, b := range g.ControlBar { + fmt.Fprintf(ctx.Output, "%s-- button %s (%s)\n", prefix, legacyItemName(b), legacyItemProps(b, false)) + } + for _, c := range g.Columns { + fmt.Fprintf(ctx.Output, "%s-- column %s (%s)\n", prefix, legacyItemName(c), legacyItemProps(c, false)) + } + for _, s := range g.SearchFields { + fmt.Fprintf(ctx.Output, "%s-- searchfield %s (%s)\n", prefix, legacyItemName(s), legacyItemProps(s, true)) + } +} + +func legacyItemName(it rawLegacyGridItem) string { + if it.Name == "" { + return "" + } + return mdlIdent(it.Name) +} + +func legacyItemProps(it rawLegacyGridItem, withType bool) string { + props := []string{it.Kind} + if it.Caption != "" { + props = append(props, "Caption: "+mdlQuoted(legacyCommentText(it.Caption))) + } + if it.Attribute != "" { + props = append(props, "Attribute: "+it.Attribute) + } + if withType && it.Type != "" { + props = append(props, "Type: "+it.Type) + } + return strings.Join(props, ", ") +} + +// legacyCommentText keeps a value on its comment line: a line break in a +// caption would end the comment and leave the rest as a statement. +func legacyCommentText(s string) string { + return strings.NewReplacer("\r\n", " ", "\n", " ", "\r", " ").Replace(s) +} diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index a394329614..99e8a5ded1 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -1160,6 +1160,9 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { formatWidgetProps(ctx.Output, prefix, header, props, "\n") } + case "Forms$DataGrid": + outputLegacyGridV3(ctx, prefix, w) + default: // A widget MDL cannot spell. Emitted as a comment naming it, and // labelled with what that costs: describe output is re-executable, so a diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 3caf190e20..b2c33d6513 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -709,6 +709,15 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s } return []rawWidget{widget} + case "Forms$DataGrid": + // Unauthorable, but its parts are addressable by their stored names + // (mendixlabs/mxcli#1459); outputLegacyGridV3 prints them as comments. + if ds, ok := w["DataSource"].(map[string]any); ok { + widget.DataSource = parseDataSource(ds) + } + widget.LegacyGrid = parseLegacyGrid(ctx, w) + return []rawWidget{widget} + case "Forms$Label", "Pages$Label": widget.Content = extractTextCaption(ctx, w) return []rawWidget{widget} diff --git a/mdl/executor/describe_legacy_datagrid_1459_test.go b/mdl/executor/describe_legacy_datagrid_1459_test.go new file mode 100644 index 0000000000..552fe80635 --- /dev/null +++ b/mdl/executor/describe_legacy_datagrid_1459_test.go @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// mendixlabs/mxcli#1459: DESCRIBE printed a legacy Forms$DataGrid as one opaque +// `-- Forms$DataGrid (grid1)` line, so its columns, search fields and control-bar +// buttons had no names in the output and `alter page … drop column14` could not +// be written from it. The grid is still unauthorable (MDL-WIDGET38), so the +// structure is printed as comments — readable and addressable, never executed. + +func legacyText(s string) map[string]any { + return map[string]any{ + "$Type": "Texts$Text", + "Items": []any{int32(3), map[string]any{ + "$Type": "Texts$Translation", "LanguageCode": "en_US", "Text": s, + }}, + } +} + +func legacyAttrRef(qn string) map[string]any { + return map[string]any{"$Type": "DomainModels$AttributeRef", "Attribute": qn, "EntityRef": nil} +} + +// legacyDataGridBSON is a Forms$DataGrid in the shape Studio Pro stores it +// (property names from modelsdk/gen/pages: Columns of Forms$GridColumn with +// Caption + AttributeRef, ControlBar.Items of Forms$Grid*Button, and the search +// bar under DataSource.SearchBar.Items). +func legacyDataGridBSON() map[string]any { + return map[string]any{ + "$Type": "Forms$DataGrid", + "Name": "grid1", + "DataSource": map[string]any{ + "$Type": "Forms$GridXPathSource", + "EntityRef": map[string]any{"$Type": "DomainModels$DirectEntityRef", "Entity": "Shop.Order"}, + "SearchBar": map[string]any{ + "$Type": "Forms$SearchBar", + "Type": "FoldableOpen", + "Items": []any{int32(2), + map[string]any{ + "$Type": "Forms$ComparisonSearchField", "Name": "searchField1", + "Caption": legacyText("Number"), "Type": "Normal", "Operator": "Equal", + "AttributeRef": legacyAttrRef("Shop.Order.Number"), + }, + map[string]any{ + "$Type": "Forms$DropDownSearchField", "Name": "searchField2", + "Caption": legacyText("Status"), "Type": "Hidden", + "AttributeRef": legacyAttrRef("Shop.Order.Status"), + }, + }, + }, + "SortBar": map[string]any{"$Type": "Forms$GridSortBar", "SortItems": []any{int32(2)}}, + "XPathConstraint": "", + }, + "ControlBar": map[string]any{ + "$Type": "Forms$GridControlBar", + "Items": []any{int32(2), + map[string]any{"$Type": "Forms$GridSearchButton", "Name": "searchButton1", "Caption": legacyText("Search")}, + map[string]any{"$Type": "Forms$GridNewButton", "Name": "newButton1", "Caption": legacyText("New")}, + }, + }, + "Columns": []any{int32(2), + map[string]any{ + "$Type": "Forms$GridColumn", "Name": "column1", "Caption": legacyText("Order number"), + "AttributeRef": legacyAttrRef("Shop.Order.Number"), + }, + map[string]any{ + "$Type": "Forms$GridColumn", "Name": "column14", "Caption": legacyText("Internal note"), + "AttributeRef": legacyAttrRef("Shop.Order.Note"), + }, + }, + } +} + +func TestDescribeLegacyDataGridPrintsItsStructure(t *testing.T) { + ctx, buf := newMockCtx(t) + raw := parseRawWidget(ctx, legacyDataGridBSON()) + describeWidgetBody(ctx, func(c *ExecContext) { + for _, w := range raw { + outputWidgetMDLV3(c, w, 1) + } + }) + body := buf.String() + t.Log("\n" + body) + + for _, want := range []string{ + "grid1", "NOT re-executable", "Shop.Order", + "searchButton1", "newButton1", + "column1", "'Order number'", "Number", + "column14", "'Internal note'", "Note", + "searchField1", "Comparison", "searchField2", "DropDown", "Hidden", "Status", + } { + if !strings.Contains(body, want) { + t.Errorf("describe output lacks %q:\n%s", want, body) + } + } + // Every line is a comment: the grid cannot be built, so a describe → exec + // round trip must not create anything in its place. + for _, line := range strings.Split(strings.TrimRight(body, "\n"), "\n") { + if !strings.HasPrefix(strings.TrimSpace(line), "--") { + t.Errorf("legacy data grid line is not a comment: %q", line) + } + } + script := "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) {\n" + body + "};" + if _, errs := visitor.Build(script); len(errs) > 0 { + t.Errorf("describe output does not parse: %v\n%s", errs[0], script) + } +} From 0ae417cda55de9a71c19734b63c183e548c2febc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:51:08 +0000 Subject: [PATCH 03/23] fix(pages): describe and write a layout-grid row's Visible: (mendixlabs/mxcli#1461) A row is built outside buildWidgetV3, pages.LayoutGridRow had no slot, describe never read Forms$LayoutGridRow.ConditionalVisibilitySettings and the writer left the null default: describe -> exec DESTROYED a Studio Pro row's condition (row always visible). Now read, printed, built (shared childVisibility with tab pages) and written. Columns have no such property (0/732 stored): check warns MDL-WIDGET07. Tests fail with each fix stubbed out; describe -> exec Unchanged on pedapp/TestApp. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...-13-layoutgrid-row-visibility-dropped.json | 1 + .../mendix/create-page/reference/widgets.md | 10 ++ mdl/backend/modelsdk/widget_write.go | 6 + ...et_write_layoutgrid_row_visibility_test.go | 54 ++++++ mdl/executor/cmd_pages_builder_v3_layout.go | 31 +++- mdl/executor/cmd_pages_describe.go | 3 +- mdl/executor/cmd_pages_describe_parse.go | 3 + .../cmd_pages_layoutgrid_row_visible_test.go | 166 ++++++++++++++++++ .../validate_layoutgrid_column_visible.go | 57 ++++++ mdl/executor/validate_widgets.go | 2 + mdl/executor/widget_rule_ids_test.go | 5 +- sdk/pages/pages_widgets_container.go | 4 + 12 files changed, 332 insertions(+), 10 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-13-layoutgrid-row-visibility-dropped.json create mode 100644 mdl/backend/modelsdk/widget_write_layoutgrid_row_visibility_test.go create mode 100644 mdl/executor/cmd_pages_layoutgrid_row_visible_test.go create mode 100644 mdl/executor/validate_layoutgrid_column_visible.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-layoutgrid-row-visibility-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-layoutgrid-row-visibility-dropped.json new file mode 100644 index 0000000000..039a70ff89 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-layoutgrid-row-visibility-dropped.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-13", "symptom": "mendixlabs/mxcli#1461: DESCRIBE printed a Studio Pro layout-grid row with a visibility condition as a bare `row { … }`, and `row (Visible: …)` passed check and exec and was not written; describe -> exec (create or modify / replace) of such a page silently made every conditional row always visible, mx check clean. Visible: on a layout-grid column was dropped too, without a warning.", "cause": "Same shape as the tab page (#1465): a row is built by buildLayoutGridV3/buildLayoutGridRowV3, not buildWidgetV3, so the visibility tail never ran; sdk/pages.LayoutGridRow had no field; layoutGridRowToGen never set it, so the registered NullFields default wrote null over the stored condition on every rewrite; parseLayoutGridRows never read it. A column has no slot at all: gen Forms$LayoutGridColumn has no ConditionalVisibilitySettings and 0 of 732 Studio Pro columns (TestApp + PedApp) carry the key, while 483 of 483 rows do.", "file": "sdk/pages/pages_widgets_container.go (LayoutGridRow.ConditionalVisibility); mdl/executor/cmd_pages_builder_v3_layout.go (childVisibility, shared with buildTabPageV3; top-level row leaves it to the wrapping container); mdl/backend/modelsdk/widget_write.go (layoutGridRowToGen); mdl/executor/cmd_pages_describe_parse.go (parseLayoutGridRows -> extractConditionalSettings); mdl/executor/validate_layoutgrid_column_visible.go (MDL-WIDGET07 warning for a column inside a row)", "insight": "Scan the fixtures' stored BSON for the key per $Type before deciding row vs column: one Go walk over mprcontents settled it (rows 483/483 have the key, columns 0/732) where the issue asked for both. TestApp's only conditional row (WorkflowCommons.Snip_DashboardContext_Timeline) sits inside a pluggable timeline slot that is passed through, so the TestApp round trip never saw the loss and ALTER (raw mutator) preserves it too — the destruction is in the rebuild path only; reproduce it by writing a conditional row with the fixed binary and re-running describe -> exec with the old one (Replaced, conditions gone) vs the new one (Unchanged). A top-level `row` is a container around a one-row grid; buildWidgetV3 already puts Visible: on the container, so the row must not carry it a second time. Note MDL-WIDGET43 (datagrid column $currentObject) also fires on a layout-grid `column`, labelled datagrid column.", "refs": ["mendixlabs/mxcli#1461", "mendixlabs/mxcli#1465", "docs-wiki/bug-patterns/silent-property-drop.md"], "test": "mdl/executor/cmd_pages_layoutgrid_row_visible_test.go (TestLayoutGridRow_VisibleIsWritten, TestLayoutGridRow_TopLevelRowVisibleOnContainerOnly, TestDescribe_LayoutGridRowVisible, TestCheck_LayoutGridColumnVisibleDropped); mdl/backend/modelsdk/widget_write_layoutgrid_row_visibility_test.go (TestLayoutGridRow_WritesConditionalVisibility)"} diff --git a/.claude/skills/mendix/create-page/reference/widgets.md b/.claude/skills/mendix/create-page/reference/widgets.md index 309a4991c3..e19ddc7cf8 100644 --- a/.claude/skills/mendix/create-page/reference/widgets.md +++ b/.claude/skills/mendix/create-page/reference/widgets.md @@ -264,6 +264,16 @@ layoutgrid gridName { column (desktopwidth: 8, tabletwidth: 6, phonewidth: 12) { ... } ``` +**Row visibility:** a `row` takes `Visible:` in the same forms as a widget +(an expression, `false`, or `Attr in (…)`) and DESCRIBE prints it. A layout-grid +**column has no visibility** in Mendix (`Forms$LayoutGridColumn` stores none), so +`Visible:` on a column inside a row is dropped — check warns (MDL-WIDGET07). Put +the condition on the row, or wrap the column's content in a `container (Visible: …)`. + +```sql +row (Visible: $currentObject/Outcome != empty) { column (desktopwidth: autofill) { ... } } +``` + Example: ```sql layoutgrid mainGrid { diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index ddee9a926a..a950fdf0ab 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -1049,6 +1049,12 @@ func layoutGridRowToGen(row *pages.LayoutGridRow) (element.Element, error) { g.SetHorizontalAlignment(alignmentOrNone(row.HorizontalAlignment)) g.SetSpacingBetweenColumns(!row.NoSpacingBetweenColumns) g.SetVerticalAlignment(alignmentOrNone(row.VerticalAlignment)) + // The row's own visibility (mendixlabs/mxcli#1461). It was never set, so the + // registered null default replaced a Studio Pro row's condition on every + // rewrite of its page, the row then always visible. + if row.ConditionalVisibility != nil { + g.SetConditionalVisibilitySettings(conditionalVisibilityToGen(row.ConditionalVisibility)) + } for _, col := range row.Columns { cg, err := layoutGridColumnToGen(col) if err != nil { diff --git a/mdl/backend/modelsdk/widget_write_layoutgrid_row_visibility_test.go b/mdl/backend/modelsdk/widget_write_layoutgrid_row_visibility_test.go new file mode 100644 index 0000000000..fb8751ef47 --- /dev/null +++ b/mdl/backend/modelsdk/widget_write_layoutgrid_row_visibility_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + bsonv1 "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1461: a layout-grid row's visibility condition is stored in +// Forms$LayoutGridRow.ConditionalVisibilitySettings. layoutGridRowToGen never +// set it, so the registered null default was written for every row — a +// describe → exec (or create or replace) of a Studio Pro page whose rows were +// conditional made each of them always visible. Control: a row without a +// condition keeps Studio Pro's null. +func TestLayoutGridRow_WritesConditionalVisibility(t *testing.T) { + lg := &pages.LayoutGrid{ + BaseWidget: pages.BaseWidget{Name: "lg"}, + Rows: []*pages.LayoutGridRow{ + {}, + {ConditionalVisibility: &pages.ConditionalVisibilitySettings{Expression: "$currentObject/Outcome != empty"}}, + }, + } + el, err := widgetToGen(lg) + if err != nil { + t.Fatal(err) + } + rows := subDocs(t, encodeElement(t, el), "Rows") + field := func(doc bsonv1.D, key string) (any, bool) { + for _, e := range doc { + if e.Key == key { + return e.Value, true + } + } + return nil, false + } + if v, ok := field(rows[0], "ConditionalVisibilitySettings"); !ok || v != nil { + t.Errorf("row 1: ConditionalVisibilitySettings = %v (present %v), want Studio Pro's null", v, ok) + } + v, _ := field(rows[1], "ConditionalVisibilitySettings") + cvs, ok := v.(bsonv1.D) + if !ok { + t.Fatalf("row 2: ConditionalVisibilitySettings = %v, want the settings — the condition was dropped (row always visible)", v) + } + if typ, _ := field(cvs, "$Type"); typ != "Forms$ConditionalVisibilitySettings" { + t.Errorf("row 2: settings $Type = %v", typ) + } + if expr, _ := field(cvs, "Expression"); expr != "$currentObject/Outcome != empty" { + t.Errorf("row 2: Expression = %v", expr) + } +} diff --git a/mdl/executor/cmd_pages_builder_v3_layout.go b/mdl/executor/cmd_pages_builder_v3_layout.go index d3fded454e..d91b611b4c 100644 --- a/mdl/executor/cmd_pages_builder_v3_layout.go +++ b/mdl/executor/cmd_pages_builder_v3_layout.go @@ -71,6 +71,13 @@ func (pb *pageBuilder) buildLayoutGridRowV3(w *ast.WidgetV3) (*pages.LayoutGridR } row.NoSpacingBetweenColumns = !v } + // A row's own visibility (Forms$LayoutGridRow.ConditionalVisibilitySettings). + // Like a tab page, a row is built here and not by buildWidgetV3, whose tail + // applies Visible: to every widget, so it was parsed and dropped + // (mendixlabs/mxcli#1461). + if row.ConditionalVisibility, err = pb.childVisibility(w); err != nil { + return nil, err + } // Build columns from children for _, child := range w.Children { @@ -199,7 +206,9 @@ func (pb *pageBuilder) buildContainerWithRowV3(w *ast.WidgetV3) (*pages.Containe // The appearance written on a top-level `row` is the container's // (applyWidgetAppearance sets it there); writing it on the row too would // apply every class and design property twice. + // Its Visible: likewise (buildWidgetV3's tail sets it on the container). row.Class, row.Style, row.DynamicClasses, row.DesignProperties = "", "", "", nil + row.ConditionalVisibility = nil lg.Rows = append(lg.Rows, row) container.Widgets = append(container.Widgets, lg) @@ -342,14 +351,11 @@ func (pb *pageBuilder) buildTabPageV3(w *ast.WidgetV3) (*pages.TabPage, error) { // Visibility: a tab page bypasses buildWidgetV3, whose common tail applies // it to every widget, so it is applied here — it used to be parsed and // dropped, the tab always visible (mendixlabs/mxcli#1465). - tp.ConditionalVisibility = expressionVisibility(w) - if vw, ok := w.Properties["VisibleWhen"].(*ast.VisibleWhenV3); ok && vw != nil { - cvs, err := pb.visibleWhenSettings(w, vw) - if err != nil { - return nil, err - } - tp.ConditionalVisibility = cvs + cvs, err := pb.childVisibility(w) + if err != nil { + return nil, err } + tp.ConditionalVisibility = cvs // Build child widgets for _, child := range w.Children { @@ -511,3 +517,14 @@ func (pb *pageBuilder) buildControlBarV3(w *ast.WidgetV3) (*pages.Container, err return controlBar, nil } + +// childVisibility is the ConditionalVisibilitySettings for `Visible:` on an +// element its parent builds outside buildWidgetV3 — a tab page (#1465) or a +// layout-grid row (#1461) — in either form: an expression, or `Attr in (…)`. +// nil when there is none. +func (pb *pageBuilder) childVisibility(w *ast.WidgetV3) (*pages.ConditionalVisibilitySettings, error) { + if vw, ok := w.Properties["VisibleWhen"].(*ast.VisibleWhenV3); ok && vw != nil { + return pb.visibleWhenSettings(w, vw) + } + return expressionVisibility(w), nil +} diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index f39888d840..21ed58aaf4 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -953,7 +953,8 @@ type rawDesignProp struct { type rawWidgetRow struct { Columns []rawWidgetColumn // Appearance holds only Class, Style, DynamicClasses and DesignProperties — - // a row is not a widget, but its Forms$Appearance is a widget's. + // a row is not a widget, but its Forms$Appearance is a widget's — and the + // row's visibility (VisibleIf / VisibleAttr / VisibleValues, #1461). Appearance rawWidget VerticalAlignment string // "" or "None" is the default HorizontalAlignment string diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 3caf190e20..16ecfa974b 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -795,6 +795,9 @@ func parseLayoutGridRows(ctx *ExecContext, w map[string]any, entityContext ...st // describe → exec reset every one of them (the Atlas "Flex container" / // "Column gap" / "Cards style" a Studio Pro page sets on its grid). extractAppearance(&row.Appearance, rMap) + // A row's own visibility (mendixlabs/mxcli#1461); a row has no + // editability, and a column no visibility, in the metamodel. + extractConditionalSettings(ctx, &row.Appearance, rMap) row.VerticalAlignment, _ = rMap["VerticalAlignment"].(string) row.HorizontalAlignment, _ = rMap["HorizontalAlignment"].(string) if v, ok := rMap["SpacingBetweenColumns"].(bool); ok { diff --git a/mdl/executor/cmd_pages_layoutgrid_row_visible_test.go b/mdl/executor/cmd_pages_layoutgrid_row_visible_test.go new file mode 100644 index 0000000000..339471b6d6 --- /dev/null +++ b/mdl/executor/cmd_pages_layoutgrid_row_visible_test.go @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1461: a layout-grid row carries its own visibility +// (Forms$LayoutGridRow.ConditionalVisibilitySettings — 483 of 483 rows in +// TestApp + PedApp have the key, TestApp's Snip_DashboardContext_Timeline sets +// one). DESCRIBE printed a bare `row { … }`, and `row (Visible: …)` passed +// check and exec and was not written, so describe → exec made every such row +// always visible. A row is built by buildLayoutGridV3, not buildWidgetV3, so +// the visibility tail every widget gets never ran for it (as for a tab page, +// #1465). + +func layoutGridOf(t *testing.T, body string) *ast.WidgetV3 { + t.Helper() + src := "create page M.P (Title: 'x', Layout: A.L, Params: ($Job: M.Job)) {\n" + + " dataview dv (DataSource: $Job) {\n" + body + " }\n};\n" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v\n%s", errs, src) + } + return prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0].Children[0] +} + +func TestLayoutGridRow_VisibleIsWritten(t *testing.T) { + lgw := layoutGridOf(t, ` + layoutgrid lg { + row { column (DesktopWidth: AutoFill) { dynamictext t1 (Content: 'A') } } + row (Visible: $currentObject/IsLocal) { column (DesktopWidth: AutoFill) { dynamictext t2 (Content: 'B') } } + row (Class: 'r3', Visible: Status in (Running, empty)) { column (DesktopWidth: AutoFill) { dynamictext t3 (Content: 'C') } } + row (Visible: false) { column (DesktopWidth: AutoFill) { dynamictext t4 (Content: 'D') } } + } +`) + built, err := visibleWhenPB("M.Job").buildWidgetV3(lgw) + if err != nil { + t.Fatalf("build: %v", err) + } + rows := built.(*pages.LayoutGrid).Rows + if len(rows) != 4 { + t.Fatalf("got %d rows, want 4", len(rows)) + } + // Control: a row without Visible: stays without a setting. + if cv := rows[0].ConditionalVisibility; cv != nil { + t.Errorf("row 1: a row without Visible: got a visibility setting %+v", cv) + } + if cv := rows[1].ConditionalVisibility; cv == nil || cv.Expression != "$currentObject/IsLocal" { + t.Errorf("row 2: ConditionalVisibility = %+v, want Expression $currentObject/IsLocal — the condition was dropped (row always visible)", cv) + } + if cv := rows[2].ConditionalVisibility; cv == nil || cv.Attribute != "M.Job.Status" || len(cv.Conditions) != 4 { + t.Errorf("row 3: ConditionalVisibility = %+v, want the attribute condition on M.Job.Status", cv) + } + if rows[2].Class != "r3" { + t.Errorf("row 3: Class = %q, want r3", rows[2].Class) + } + if cv := rows[3].ConditionalVisibility; cv == nil || cv.Expression != "false" { + t.Errorf("row 4: ConditionalVisibility = %+v, want Expression false", cv) + } +} + +// A top-level `row` builds a container around a one-row grid, and the +// container carries the row's Visible: (as it carries its appearance) — it +// must not be written on the row a second time. +func TestLayoutGridRow_TopLevelRowVisibleOnContainerOnly(t *testing.T) { + roww := layoutGridOf(t, ` + row r (Visible: $currentObject/IsLocal) { column (DesktopWidth: AutoFill) { dynamictext t1 (Content: 'A') } } +`) + built, err := visibleWhenPB("M.Job").buildWidgetV3(roww) + if err != nil { + t.Fatalf("build: %v", err) + } + c, ok := built.(*pages.Container) + if !ok { + t.Fatalf("got %T, want the wrapping container", built) + } + if cv := c.ConditionalVisibility; cv == nil || cv.Expression != "$currentObject/IsLocal" { + t.Errorf("container: ConditionalVisibility = %+v, want the row's condition", cv) + } + row := c.Widgets[0].(*pages.LayoutGrid).Rows[0] + if row.ConditionalVisibility != nil { + t.Errorf("row: ConditionalVisibility = %+v, want none — the container already carries it", row.ConditionalVisibility) + } +} + +// DESCRIBE reads a row's visibility back, so describe -> exec keeps it. +// The stored shape is TestApp's WorkflowCommons.Snip_DashboardContext_Timeline. +func TestDescribe_LayoutGridRowVisible(t *testing.T) { + col := map[string]any{"$Type": "Forms$LayoutGridColumn", "Weight": int32(-1), "TabletWeight": int32(-1), "PhoneWeight": int32(-1), "Widgets": []any{int32(2)}} + stored := map[string]any{ + "$Type": "Forms$LayoutGrid", + "Name": "lg", + "Rows": []any{int32(2), + map[string]any{"$Type": "Forms$LayoutGridRow", "SpacingBetweenColumns": true, "Columns": []any{int32(2), col}}, + map[string]any{ + "$Type": "Forms$LayoutGridRow", "SpacingBetweenColumns": true, "Columns": []any{int32(2), col}, + "ConditionalVisibilitySettings": map[string]any{ + "$Type": "Forms$ConditionalVisibilitySettings", + "Expression": "$currentObject/Outcome != empty", + }, + }, + }, + } + var buf bytes.Buffer + ctx := (&Executor{}).newExecContext(context.Background()) + ctx.Output = &buf + raw := parseRawWidget(ctx, stored) + outputWidgetMDLV3(ctx, raw[0], 1) + got := buf.String() + if !strings.Contains(got, "row (Visible: $currentObject/Outcome != empty) {") { + t.Errorf("describe output lacks the row's Visible:\n%s", got) + } + if strings.Count(got, "Visible:") != 1 { + t.Errorf("only the second row has a visibility setting:\n%s", got) + } + if _, errs := visitor.Build("create page M.P (Title: 'x', Layout: A.L) {\n" + got + "}\n"); len(errs) > 0 { + t.Errorf("describe output does not parse: %v\n%s", errs, got) + } +} + +// Forms$LayoutGridColumn has no ConditionalVisibilitySettings (none of 732 +// stored columns in TestApp + PedApp has the key; the 11.x metamodel has no +// such property), so `Visible:` on a column inside a row is dropped. check +// says so. Controls: the row's own Visible:, and a top-level `column`, whose +// wrapping container carries it, draw nothing. +func TestCheck_LayoutGridColumnVisibleDropped(t *testing.T) { + src := `create page M.P (Title: 'x', Layout: A.L, Params: ($Job: M.Job)) { + dataview dv (DataSource: $Job) { + layoutgrid lg { + row (Visible: $currentObject/IsLocal) { + column c1 (DesktopWidth: AutoFill, Visible: $Job/IsLocal) { dynamictext t1 (Content: 'A') } + column (DesktopWidth: AutoFill) { dynamictext t2 (Content: 'B') } + } + } + column c3 (DesktopWidth: AutoFill, Visible: $currentObject/IsLocal) { dynamictext t3 (Content: 'C') } + } +}; +` + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + reg, _ := NewWidgetRegistry() + var hits []linter.Violation + for _, v := range ValidateWidgetPropertiesForStatement(prog.Statements[0], reg) { + if strings.Contains(v.Message, "layout-grid column") { + hits = append(hits, v) + } + } + if len(hits) != 1 { + t.Fatalf("got %d layout-grid column visibility violations, want 1 (c1): %+v", len(hits), hits) + } + if hits[0].RuleID != "MDL-WIDGET07" || hits[0].Severity != linter.SeverityWarning || !strings.Contains(hits[0].Message, "column 1 of its row") { + t.Errorf("got %s/%v %q, want an MDL-WIDGET07 warning naming column 1", hits[0].RuleID, hits[0].Severity, hits[0].Message) + } +} diff --git a/mdl/executor/validate_layoutgrid_column_visible.go b/mdl/executor/validate_layoutgrid_column_visible.go new file mode 100644 index 0000000000..86cfefbc61 --- /dev/null +++ b/mdl/executor/validate_layoutgrid_column_visible.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// validateLayoutGridColumnVisible warns about `Visible:` on a column inside a +// layout-grid row. Forms$LayoutGridColumn has no ConditionalVisibilitySettings +// — the metamodel has no such property, and none of the 732 Studio +// Pro-authored columns in TestApp + PedApp carries the key — so +// buildLayoutGridColumnV3 has nowhere to put it and it is dropped. `Visible` is +// in the cross-widget vocabulary, so MDL-WIDGET07 never saw it +// (mendixlabs/mxcli#1461). The row has visibility; a top-level `column` is +// wrapped in a container, which carries it, so neither is reported. +// +// A warning, as MDL-WIDGET07 is, so no language version is needed: the write +// is unchanged, only no longer silent. +func validateLayoutGridColumnVisible(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + if w == nil || !strings.EqualFold(w.Type, "row") { + return nil + } + var out []linter.Violation + n := 0 + for _, col := range w.Children { + if col == nil || !strings.EqualFold(col.Type, "column") { + continue + } + n++ // Mendix stores no name on a column, so it is named by position. + // Visible: in any form — expression (Visible / VisibleIf) or + // `Attr in (…)` (VisibleWhen). + has := false + for _, key := range sortedPropertyKeys(col) { + switch strings.ToLower(key) { + case "visible", "visibleif", "visiblewhen": + has = true + } + } + if !has { + continue + } + out = append(out, linter.Violation{ + RuleID: "MDL-WIDGET07", + Severity: linter.SeverityWarning, + Message: fmt.Sprintf("%s: layout-grid column %d of its row: `Visible` cannot be stored on a layout-grid column "+ + "(Forms$LayoutGridColumn has no visibility setting) and will be silently dropped on write", + locationPrefix, n), + Suggestion: "put Visible: on the row, or wrap the column's content in a `container` with Visible:", + }) + } + return out +} diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 3b6a50fc32..f85d8797dd 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -250,6 +250,8 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc out = append(out, validateLegacyExpressionText(w, locationPrefix)...) // A column's Visible has no row object (MDL-WIDGET43, CE0117). out = append(out, validateColumnVisibleScope(w, locationPrefix)...) + // …and a layout-grid column's Visible has nowhere to be stored (#1461). + out = append(out, validateLayoutGridColumnVisible(w, locationPrefix)...) // #1062: an action slot holding something that is not an action, which // used to check clean, exec clean, build clean and render dead. Runs for // every widget kind and needs no definition, for the same reason as the diff --git a/mdl/executor/widget_rule_ids_test.go b/mdl/executor/widget_rule_ids_test.go index cb2cdf564e..005e474fdc 100644 --- a/mdl/executor/widget_rule_ids_test.go +++ b/mdl/executor/widget_rule_ids_test.go @@ -38,8 +38,9 @@ var widgetRuleIDsRaisedFromSeveralFiles = map[string][]string{ // One rule about a property the builder silently drops on write — a key no // builder reads, or DesignProperties on a slot of the parent (a layoutgrid // row, a row's column, a dataview footer) that never reaches - // applyWidgetAppearance. - "MDL-WIDGET07": {"validate_design_properties.go", "validate_widgets.go"}, + // applyWidgetAppearance, or Visible: on a row's column, which has no + // visibility slot (mendixlabs/mxcli#1461). + "MDL-WIDGET07": {"validate_design_properties.go", "validate_layoutgrid_column_visible.go", "validate_widgets.go"}, } // Matched as a QUOTED literal rather than after `RuleID:`, because an id is diff --git a/sdk/pages/pages_widgets_container.go b/sdk/pages/pages_widgets_container.go index a326a7ebce..508471e322 100644 --- a/sdk/pages/pages_widgets_container.go +++ b/sdk/pages/pages_widgets_container.go @@ -36,6 +36,10 @@ type LayoutGridRow struct { VerticalAlignment string `json:"verticalAlignment,omitempty"` // "None" (default), "Start", "Center", "End" HorizontalAlignment string `json:"horizontalAlignment,omitempty"` // "None" (default), "Start", "Center", "End" NoSpacingBetweenColumns bool `json:"noSpacingBetweenColumns,omitempty"` + // ConditionalVisibility is the row's own visibility condition + // (Forms$LayoutGridRow.ConditionalVisibilitySettings); nil writes Studio + // Pro's null (mendixlabs/mxcli#1461). A column has no such slot. + ConditionalVisibility *ConditionalVisibilitySettings `json:"conditionalVisibility,omitempty"` } // LayoutGridColumn represents a column in a layout grid. Like a row it has an From c8ba7fb1f2a3c842ef011c6689e77595af55b9ef Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:51:22 +0000 Subject: [PATCH 04/23] fix(widgets): native widgets no longer take over image/radiobuttons (mendixlabs/mxcli#1463) widget init named native widgets after the last id segment (image, radiobuttons), and the registry let a different-id definition replace a built-in keyword; check resolved through the registry while the builder's switch owns `radiobuttons`. Native widgets are now named native, the registry refuses a different-id def under a built-in/builder-owned keyword (warning, still reachable by id), and lookupWidgetDef skips builder-owned keywords (set pinned to the switch by a go/ast test). Reverting the guards brings back "has no property Image" and the native radiobuttons resolution. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...s-over-web-image-radiobuttons-keyword.json | 1 + .claude/skills/mendix/custom-widgets/SKILL.md | 10 + mdl/executor/validate_widgets.go | 5 +- mdl/executor/widget_defs.go | 18 +- mdl/executor/widget_defs_test.go | 5 + mdl/executor/widget_keyword_shadow_test.go | 207 ++++++++++++++++++ mdl/executor/widget_keywords.go | 34 +++ mdl/executor/widget_registry.go | 20 ++ 8 files changed, 297 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-13-native-widget-takes-over-web-image-radiobuttons-keyword.json create mode 100644 mdl/executor/widget_keyword_shadow_test.go create mode 100644 mdl/executor/widget_keywords.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-native-widget-takes-over-web-image-radiobuttons-keyword.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-native-widget-takes-over-web-image-radiobuttons-keyword.json new file mode 100644 index 0000000000..8cc36f03db --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-native-widget-takes-over-web-image-radiobuttons-keyword.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-13", "symptom": "with native mobile widget packages installed, `check` reports `image img1 (Image: …)` \"has no property Image [MDL-WIDGET01]\" and validates `radiobuttons` against the native RadioButtons, while exec writes a Forms$RadioButtonGroup", "cause": "DeriveMDLName took only the last id segment, so `widget init` wrote image.def.json / radiobuttons.def.json for com.mendix.widget.native.*; the registry let a user def of a DIFFERENT widget id replace a built-in name (info log only), and check resolves keywords through the registry while the builder's switch owns `radiobuttons` before any lookup", "file": "`mdl/executor/widget_registry.go` (`loadDefinitionsFromDir`), `widget_keywords.go` (`builderOwnedWidgetKeywords`), `validate_widgets.go` (`lookupWidgetDef`), `widget_defs.go` (`DeriveMDLName`)", "insight": "Fixing only the init-time name leaves the bad files already on users' disks, so the guard has to sit where definitions are loaded: refuse a different-id def under a built-in or builder-owned keyword, keep it reachable by id. The builder-owned keyword set is pinned to buildWidgetV3's switch by a go/ast test, so check and exec cannot drift again. DESCRIBE needed nothing: pluggableWidgetHeader already round-trips the name through Get and falls back to `pluggablewidget ''`", "refs": ["mendixlabs/mxcli#1463"]} diff --git a/.claude/skills/mendix/custom-widgets/SKILL.md b/.claude/skills/mendix/custom-widgets/SKILL.md index c321514000..fb82242c1b 100644 --- a/.claude/skills/mendix/custom-widgets/SKILL.md +++ b/.claude/skills/mendix/custom-widgets/SKILL.md @@ -146,6 +146,16 @@ If a widget you have installed is not found, extract its definition: mxcli widget init -p app.mpr ``` +The keyword is the last segment of the widget id, lowercased — except for a +**native mobile** widget (`com.mendix.widget.native.*`), which is prefixed +`native`: `nativeimage`, `nativeradiobuttons`, `nativebadge`. Native packages +reuse the web names, and a bare `image`/`radiobuttons` used to take over the +web keyword (mendixlabs/mxcli#1463). A project definition never takes a +built-in keyword (`image`, `radiobuttons`, `textbox`, …) for a different +widget id: it is reported with a `warning:` line and stays reachable only as +`pluggablewidget ''` — delete an old `image.def.json` / +`radiobuttons.def.json` generated for a native widget and re-run `widget init`. + ## Built-in Pluggable Widgets ### GALLERY diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 3b6a50fc32..22252ca175 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -1577,7 +1577,10 @@ func lookupWidgetDef(w *ast.WidgetV3, registry *WidgetRegistry) *WidgetDefinitio return def } } - if w.Type == "" { + // A keyword the page builder builds itself never reaches the registry + // there, so check must not resolve it through the registry either + // (mendixlabs/mxcli#1463: `radiobuttons`). + if w.Type == "" || isBuilderOwnedWidgetKeyword(w.Type) { return nil } if def, ok := registry.Get(strings.ToUpper(w.Type)); ok { diff --git a/mdl/executor/widget_defs.go b/mdl/executor/widget_defs.go index 7882892b36..bf21310781 100644 --- a/mdl/executor/widget_defs.go +++ b/mdl/executor/widget_defs.go @@ -222,10 +222,24 @@ func RefreshStaleWidgetDefinitions(projectPath string) (bool, error) { // DeriveMDLName derives the uppercase MDL keyword name from a widget ID // (e.g. "com.mendix.widget.web.accordion.Accordion" → "ACCORDION"). +// +// A native mobile widget is prefixed with NATIVE +// ("com.mendix.widget.native.image.Image" → "NATIVEIMAGE"): Mendix ships most +// of them under the same last segment as a web widget or a built-in keyword +// (image, radiobuttons, badge, …), and the bare name let the native package +// take over the web keyword (mendixlabs/mxcli#1463). func DeriveMDLName(widgetID string) string { parts := strings.Split(widgetID, ".") - name := parts[len(parts)-1] - return strings.ToUpper(name) + name := strings.ToUpper(parts[len(parts)-1]) + for _, seg := range parts[:len(parts)-1] { + if strings.EqualFold(seg, "native") { + if !strings.HasPrefix(name, "NATIVE") { + name = "NATIVE" + name + } + break + } + } + return name } // GenerateDefJSON creates a skeleton WidgetDefinition from an mpk.WidgetDefinition. diff --git a/mdl/executor/widget_defs_test.go b/mdl/executor/widget_defs_test.go index 90586fbab0..cee81eb268 100644 --- a/mdl/executor/widget_defs_test.go +++ b/mdl/executor/widget_defs_test.go @@ -22,6 +22,11 @@ func TestDeriveMDLName(t *testing.T) { {"com.mendix.widget.web.gallery.Gallery", "GALLERY"}, {"com.company.widget.MyCustomWidget", "MYCUSTOMWIDGET"}, {"SimpleWidget", "SIMPLEWIDGET"}, + // mendixlabs/mxcli#1463: native widgets must not take the web + // keyword (`image`, `radiobuttons`, `badge`, …). + {"com.mendix.widget.native.image.Image", "NATIVEIMAGE"}, + {"com.mendix.widget.native.radiobuttons.RadioButtons", "NATIVERADIOBUTTONS"}, + {"com.mendix.widget.web.image.Image", "IMAGE"}, } for _, tc := range tests { diff --git a/mdl/executor/widget_keyword_shadow_test.go b/mdl/executor/widget_keyword_shadow_test.go new file mode 100644 index 0000000000..f262920e61 --- /dev/null +++ b/mdl/executor/widget_keyword_shadow_test.go @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "go/ast" + "go/parser" + "go/token" + "log" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" + + mdlast "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1463: a project with the native mobile widget packages +// installed got `image.def.json` and `radiobuttons.def.json` from `widget init` +// (the MDL name was the last segment of the widget id), and those took over the +// built-in web keywords. `check` then validated `image img1 (Image: …)` against +// the NATIVE Image — "has no property Image [MDL-WIDGET01]" — and validated +// `radiobuttons` against the native RadioButtons while the builder, which owns +// that keyword in its switch, wrote a Forms$RadioButtonGroup. +// +// The definitions below are the shape such a generated file has on a user's +// disk: the fix has to hold for files that already exist, not only for the +// names the next `widget init` derives. +const nativeImageDef = `{"widgetId":"com.mendix.widget.native.image.Image","mdlName":"IMAGE","widgetKind":"pluggable","templateFile":"image.json","defaultEditable":"Always", + "propertyMappings":[{"propertyKey":"defaultImageDynamic","source":"DefaultImage","operation":"image"}]}` + +const nativeRadioButtonsDef = `{"widgetId":"com.mendix.widget.native.radiobuttons.RadioButtons","mdlName":"RADIOBUTTONS","widgetKind":"pluggable","templateFile":"radiobuttons.json","defaultEditable":"Always", + "propertyMappings":[{"propertyKey":"enum","source":"Attribute","operation":"attribute"}]}` + +func writeShadowingProject(t *testing.T) string { + t.Helper() + dir := t.TempDir() + widgets := filepath.Join(dir, ".mxcli", "widgets") + if err := os.MkdirAll(widgets, 0o755); err != nil { + t.Fatal(err) + } + for name, body := range map[string]string{ + "image.def.json": nativeImageDef, + "radiobuttons.def.json": nativeRadioButtonsDef, + } { + if err := os.WriteFile(filepath.Join(widgets, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + return filepath.Join(dir, "App.mpr") +} + +func TestUserDefinitionDoesNotShadowBuiltinKeyword(t *testing.T) { + projectPath := writeShadowingProject(t) + reg, err := NewWidgetRegistry() + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + log.SetOutput(&buf) + defer log.SetOutput(os.Stderr) + if err := reg.LoadUserDefinitions(projectPath); err != nil { + t.Fatal(err) + } + + if def, ok := reg.Get("image"); !ok || def.WidgetID != "com.mendix.widget.web.image.Image" { + got := "" + if ok { + got = def.WidgetID + } + t.Errorf("`image` must stay the web Image; resolves to %s", got) + } + if def, ok := reg.Get("radiobuttons"); ok { + t.Errorf("`radiobuttons` is the builder's Forms$RadioButtonGroup; the registry resolves it to %s", def.WidgetID) + } + // The native widgets stay reachable by id (`pluggablewidget '…'`). + for _, id := range []string{"com.mendix.widget.native.image.Image", "com.mendix.widget.native.radiobuttons.RadioButtons"} { + if _, ok := reg.GetByWidgetID(id); !ok { + t.Errorf("%s is no longer reachable by widget id", id) + } + } + if !strings.Contains(buf.String(), "warning:") { + t.Errorf("the refused keyword must be reported as a warning, got log: %q", buf.String()) + } +} + +// Control: a user definition for the SAME widget id as a built-in is a +// deliberate override and still wins. +func TestUserDefinitionSameWidgetIDStillOverridesBuiltin(t *testing.T) { + dir := t.TempDir() + widgets := filepath.Join(dir, ".mxcli", "widgets") + if err := os.MkdirAll(widgets, 0o755); err != nil { + t.Fatal(err) + } + body := `{"widgetId":"com.mendix.widget.web.image.Image","mdlName":"image","templateFile":"image.json","defaultEditable":"Always", + "propertyMappings":[{"propertyKey":"marker","source":"Attribute","operation":"attribute"}]}` + if err := os.WriteFile(filepath.Join(widgets, "image.def.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + reg, err := NewWidgetRegistry() + if err != nil { + t.Fatal(err) + } + if err := reg.LoadUserDefinitions(filepath.Join(dir, "App.mpr")); err != nil { + t.Fatal(err) + } + def, ok := reg.Get("image") + if !ok || len(def.PropertyMappings) != 1 || def.PropertyMappings[0].PropertyKey != "marker" { + t.Errorf("a same-id user override of `image` must still replace the built-in, got %+v", def) + } +} + +// The reported symptom, at the layer `check` runs it. +func TestCheckImageKeywordNotValidatedAgainstNativeImage(t *testing.T) { + reg := LoadWidgetRegistry(writeShadowingProject(t)) + if reg == nil { + t.Fatal("registry not available") + } + img := &mdlast.WidgetV3{Name: "img1", Type: "image", Properties: map[string]any{"Image": "Atlas_Core.Content.mendix"}} + for _, v := range validatePluggableWidgetProperties(img, reg, "page P") { + if v.RuleID == "MDL-WIDGET01" { + t.Errorf("web `image` validated against the native Image: %s", v.Message) + } + } + // `radiobuttons` is built by the builder's own switch; check must not + // resolve it to any pluggable definition either. + rb := &mdlast.WidgetV3{Name: "rb", Type: "radiobuttons", Properties: map[string]any{"Attribute": "Status"}} + if def := lookupWidgetDef(rb, reg); def != nil { + t.Errorf("check resolves `radiobuttons` to %s; the builder writes Forms$RadioButtonGroup", def.WidgetID) + } +} + +// DESCRIBE must print a header that reads back to the same widget: a native +// widget whose stored definition still claims a built-in keyword falls back to +// the explicit id form. +func TestDescribeNativeWidgetHeaderReadsBack(t *testing.T) { + reg := LoadWidgetRegistry(writeShadowingProject(t)) + if reg == nil { + t.Fatal("registry not available") + } + for id, want := range map[string]string{ + "com.mendix.widget.native.image.Image": "pluggablewidget 'com.mendix.widget.native.image.Image' w", + "com.mendix.widget.native.radiobuttons.RadioButtons": "pluggablewidget 'com.mendix.widget.native.radiobuttons.RadioButtons' w", + "com.mendix.widget.web.image.Image": "image w", + } { + if got := pluggableWidgetHeader(reg, id, "w"); got != want { + t.Errorf("header for %s = %q, want %q", id, got, want) + } + } +} + +// builderOwnedWidgetKeywords must list exactly the keywords buildWidgetV3's +// switch handles itself (bar `image`, which it hands to the registry). If the +// two drift, check and exec resolve a keyword differently again. +func TestBuilderOwnedWidgetKeywordsMatchSwitch(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "cmd_pages_builder_v3.go", nil, 0) + if err != nil { + t.Fatal(err) + } + var fromSwitch []string + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name != "buildWidgetV3" { + continue + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + sw, ok := n.(*ast.SwitchStmt) + if !ok { + return true + } + call, ok := sw.Tag.(*ast.CallExpr) + if !ok { + return true + } + if sel, ok := call.Fun.(*ast.SelectorExpr); !ok || sel.Sel.Name != "ToLower" { + return true + } + for _, stmt := range sw.Body.List { + for _, e := range stmt.(*ast.CaseClause).List { + if lit, ok := e.(*ast.BasicLit); ok { + s, _ := strconv.Unquote(lit.Value) + if s != "image" { + fromSwitch = append(fromSwitch, s) + } + } + } + } + return false + }) + } + if len(fromSwitch) == 0 { + t.Fatal("found no keyword switch in buildWidgetV3") + } + var fromSet []string + for k := range builderOwnedWidgetKeywords { + fromSet = append(fromSet, k) + } + sort.Strings(fromSwitch) + sort.Strings(fromSet) + if strings.Join(fromSwitch, ",") != strings.Join(fromSet, ",") { + t.Errorf("builderOwnedWidgetKeywords drifted from buildWidgetV3's switch:\n switch: %v\n set: %v", fromSwitch, fromSet) + } +} diff --git a/mdl/executor/widget_keywords.go b/mdl/executor/widget_keywords.go new file mode 100644 index 0000000000..d038d2e349 --- /dev/null +++ b/mdl/executor/widget_keywords.go @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "strings" + +// builderOwnedWidgetKeywords are the widget keywords buildWidgetV3 builds with +// its own switch case, before (and instead of) any widget-registry lookup. +// `image` is absent on purpose: its case resolves through the registry. +// +// The registry must never answer for one of these names: check resolves a +// keyword through the registry (lookupWidgetDef), so a definition registered +// under `radiobuttons` made check validate a pluggable widget while exec wrote +// a Forms$RadioButtonGroup (mendixlabs/mxcli#1463). +// TestBuilderOwnedWidgetKeywordsMatchSwitch keeps this in step with the switch. +var builderOwnedWidgetKeywords = map[string]bool{ + "dataview": true, "legacydatagrid": true, "listview": true, "layoutgrid": true, + "row": true, "column": true, "container": true, "customcontainer": true, + "slot": true, "textbox": true, "textarea": true, "datepicker": true, + "dropdown": true, "checkbox": true, "filemanager": true, "text": true, + "statictext": true, "dynamictext": true, "title": true, "label": true, + "button": true, "actionbutton": true, "linkbutton": true, "tabcontainer": true, + "tabpage": true, "groupbox": true, "scrollcontainer": true, "region": true, + "navigationtree": true, "menubar": true, "simplemenubar": true, + "placeholder": true, "radiobuttons": true, "navigationlist": true, "item": true, + "snippetcall": true, "footer": true, "header": true, "controlbar": true, + "template": true, "filter": true, "staticimage": true, "dynamicimage": true, +} + +// isBuilderOwnedWidgetKeyword reports whether keyword is built by the page +// builder's own switch rather than through the widget registry. +func isBuilderOwnedWidgetKeyword(keyword string) bool { + return builderOwnedWidgetKeywords[strings.ToLower(keyword)] +} diff --git a/mdl/executor/widget_registry.go b/mdl/executor/widget_registry.go index 57ed8a5188..035df6208b 100644 --- a/mdl/executor/widget_registry.go +++ b/mdl/executor/widget_registry.go @@ -19,6 +19,7 @@ type WidgetRegistry struct { byMDLName map[string]*WidgetDefinition // keyed by uppercase MDLName byWidgetID map[string]*WidgetDefinition // keyed by widgetId knownOperations map[string]bool // operations accepted during validation + embeddedIDs map[string]string // uppercase MDLName → widgetId of the embedded (built-in) definition projectPath string // set by LoadWidgetRegistry; lets validation resolve editorConfig visibility rules for built-in widgets whose .def.json carries none } @@ -71,6 +72,7 @@ func NewWidgetRegistryWithOps(extraOps map[string]bool) (*WidgetRegistry, error) byMDLName: make(map[string]*WidgetDefinition), byWidgetID: make(map[string]*WidgetDefinition), knownOperations: ops, + embeddedIDs: make(map[string]string), } entries, err := definitions.EmbeddedFS.ReadDir(".") @@ -103,6 +105,7 @@ func NewWidgetRegistryWithOps(extraOps map[string]bool) (*WidgetRegistry, error) } reg.byMDLName[strings.ToUpper(def.MDLName)] = &def reg.byWidgetID[def.WidgetID] = &def + reg.embeddedIDs[strings.ToUpper(def.MDLName)] = def.WidgetID } return reg, nil @@ -201,6 +204,23 @@ func (r *WidgetRegistry) loadDefinitionsFromDir(dir string) error { def.ChildSlots[i].MDLContainer = strings.ToLower(def.ChildSlots[i].MDLContainer) } upperName := strings.ToUpper(def.MDLName) + // A definition for a DIFFERENT widget must never take a built-in + // keyword: `widget init` used to name the native Image `image` and the + // native RadioButtons `radiobuttons`, and those files are still on + // users' disks (mendixlabs/mxcli#1463). Keep the widget reachable by id + // and leave the keyword with the widget it documents. + builtinID, isEmbedded := r.embeddedIDs[upperName] + if isBuilderOwnedWidgetKeyword(def.MDLName) || (isEmbedded && builtinID != def.WidgetID) { + builtin := "a built-in Forms widget" + if isEmbedded { + builtin = builtinID + } + log.Printf("warning: widget definition %q (%s) is not registered as `%s` — that keyword is %s. "+ + "Use `pluggablewidget '%s'`, or give the definition another mdlName (or delete the file and re-run `mxcli widget init`)", + filePath, def.WidgetID, def.MDLName, builtin, def.WidgetID) + r.byWidgetID[def.WidgetID] = &def + continue + } if existing, ok := r.byMDLName[upperName]; ok { // Skip user skeleton definitions (no mappings/modes) when built-in has mappings if len(def.PropertyMappings) == 0 && len(def.Modes) == 0 && From 40a1d6caa99b84f895c0640ca2a73ae7e50fef29 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:54:56 +0000 Subject: [PATCH 05/23] fix(executor): MDL055 covers widget datasource XPath (mendixlabs/mxcli#1466) Measured on fresh 11.13.0 and 11.14.0 projects: the reported one-hop form [Mod.A = $currentObject/Mod.B] builds at 0 errors in a combobox, list view and data grid 2 (stored verbatim), as do the inverted form and $currentObject/Attr. Two hops ($currentObject/Mod.B/Name) is CE0161 in combobox, list view, data grid 2 and gallery, and check passed it: MDL055 only saw microflow retrieves. The page validator now applies the same hop-count rule (shared matcher). TestDatasourceXPathVariableTraversal fails with the call stubbed out (4 flagged cases unflagged) and passes with it; the one-hop controls stay unflagged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...ath-two-hops-off-currentobject-ce0161.json | 1 + CHANGELOG.md | 1 + ...datasource-xpath-variable-traversal-ok.mdl | 49 ++++++++++ ...tasource-xpath-variable-traversal.fail.mdl | 34 +++++++ .../validate_datasource_vartraversal_test.go | 98 +++++++++++++++++++ mdl/executor/validate_microflow.go | 31 ++++-- mdl/executor/validate_widgets.go | 33 +++++++ 7 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-13-widget-datasource-xpath-two-hops-off-currentobject-ce0161.json create mode 100644 mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal-ok.mdl create mode 100644 mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal.fail.mdl create mode 100644 mdl/executor/validate_datasource_vartraversal_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-widget-datasource-xpath-two-hops-off-currentobject-ce0161.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-widget-datasource-xpath-two-hops-off-currentobject-ce0161.json new file mode 100644 index 0000000000..bf574dc837 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-widget-datasource-xpath-two-hops-off-currentobject-ce0161.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-13", "symptom": "A widget database datasource `where [Name = $currentObject/Mod.Assoc/Name]` passes `mxcli check` and `exec`, then `mx check` reports **CE0161** \"Error(s) in XPath constraint.\" at the widget (combobox, list view, data grid 2, gallery). Reported as the ONE-hop form `[Mod.District_Region = $currentObject/Mod.AreaPicker_Region]` on a combo box", "cause": "MDL055 (#831) ran only over microflow retrieve constraints; the page side had MDL047 (`= empty`) but no hop-count check, so the two-hop comparand reached mxbuild", "file": "`mdl/executor/validate_widgets.go` (`validateDatasourceXPathVariableTraversal`), `mdl/executor/validate_microflow.go` (`xpathVariableTraversals`, shared with the retrieve rule)", "insight": "**The reported form was valid: measure the issue's own script before writing the rule it asks for.** On fresh 11.13.0 and 11.14.0 projects the one-hop `$currentObject/Mod.Assoc` comparand built at 0 errors in a combobox (`Association:` and `Attribute:` spellings), list view and data grid 2, and is stored verbatim (bson dump), so neither a refusal nor a writer fix was right for it; a rule flagging one hop would refuse valid MDL. The reduced script's only errors were CE0642 \"Property 'Caption' is required\", one per combo box, because it omits `CaptionAttribute:`. Shapes that DID give CE0161 on 11.14.0 with check passing, one page each in one project: two hops off `$currentObject` (all four list widgets; now MDL055), a one-hop comparand whose target entity differs from the left side's (`[Mod.District_Region = $currentObject/Mod.AreaPicker_District]`), and an UNQUALIFIED association in the comparand (`$currentObject/AreaPicker_Region`). The last two are still unflagged and are candidates for the reporter's real app. Do not measure on testdata/pedapp: its stale widget definitions turn every new combobox into CE0642 Attribute/Values/Caption noise; `mxcli new --theme none --layout none --skip-init --skip-build` gives a 0-error control.", "refs": ["mendixlabs/mxcli#1466", "#831"], "ce": ["CE0161", "CE0642"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 529cdf303d..2365f748c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`mxcli search` and `SEARCH` find words that appear only in an element's documentation** (mendixlabs/mxcli#1477) — the strings index held captions, messages, URLs and the documentation of microflows and workflows, but not the documentation of entities, attributes, associations, enumerations, constants, pages and the other documents whose catalog tables carry it, so a word present only there answered "No matches found." (measured: a word in a constant's documentation was found by `select … from catalog.constants where Description like …` and not by `search`). The index now also holds those: each hit names the element (`ObjectType` `ENTITY`, `ATTRIBUTE`, `CONSTANT`, `PAGE`, …; an attribute is `Module.Entity.Attribute`) and the field (`StringContext` `documentation`), the labels microflow documentation already used, plus the parameters of microflows and Java actions and a module's domain model documentation (`domain_model_documentation`). A module's own rows carry the module's name in `ModuleName`, not the empty string the `objects` view holds for a module. They are read from the tables the catalog builder has just filled, not from a second walk of the model; building a catalog of a stock 11.12 app takes the same time (0.5s) and the strings table grows by about 50 rows. Enumeration values are not indexed (Mendix has no documentation field on them), nor is the summary text of a remote service's contract entities. The catalog schema version is bumped to 25, so a cached catalog is rebuilt once. Rules: docs-site "Full-Text Search". - **`search` finds a microflow's log message** (mendixlabs/mxcli#1477) — only the log node name was indexed. A log message is a plain-text `Microflows$StringTemplate`, not a `Texts$Text`, so the walk that finds every translatable text never saw it, and a word present only in a log message was not found. It is now a `log_message` row beside the `log_node` one (activities inside a loop are not reached, as before for the node name). +- **`check` refuses a widget datasource constraint that reaches two hops off a variable** (mendixlabs/mxcli#1466) — `database … where [Name = $currentObject/Mod.Assoc/Name]` on a combobox, list view, data grid 2 or gallery passed `check` and `exec`, then failed the build with CE0161. MDL055 covered only microflow retrieves; it now covers page datasources too. One hop (`[Mod.A = $currentObject/Mod.B]`), the context object's own attribute and the inverted form all build at 0 errors on 11.13.0 and 11.14.0 and stay accepted. ## [0.26.0] - 2026-10-11 diff --git a/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal-ok.mdl b/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal-ok.mdl new file mode 100644 index 0000000000..b6abcceb3d --- /dev/null +++ b/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal-ok.mdl @@ -0,0 +1,49 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1466 — controls for the page-datasource arm of MDL055 +-- ============================================================================ +-- +-- Every page here MUST pass `mxcli check`. Each was exec'd into a fresh 11.13.0 +-- and a fresh 11.14.0 project and `mx check` reported 0 errors, so a rule that +-- flags any of them refuses MDL that builds: +-- +-- [Mod.A = $currentObject/Mod.B] one hop — the form the issue reported +-- [Mod.A/Mod.E/Mod.B = $currentObject] the inverted form +-- [Name = $currentObject/Label] the context object's own attribute +-- +-- The two-hop form (`$currentObject/Mod.B/Name`) is the CE0161 one, in +-- 1466-datasource-xpath-variable-traversal.fail.mdl. +-- ============================================================================ + +create module Bug1466; +create persistent entity Bug1466.Region (Name: String(100)); +create persistent entity Bug1466.District (Name: String(100)); +create persistent entity Bug1466.AreaPicker (Label: String(50)); +create association Bug1466.District_Region from Bug1466.District to Bug1466.Region type Reference; +create association Bug1466.AreaPicker_Region from Bug1466.AreaPicker to Bug1466.Region type Reference; +create association Bug1466.AreaPicker_District from Bug1466.AreaPicker to Bug1466.District type Reference; + +create page Bug1466.Controls ( + Params: ($Picker: Bug1466.AreaPicker), Title: 'Area', Layout: Atlas_Core.Atlas_Default +) { + layoutgrid lg { + row { + column (desktopwidth: autofill) { + dataview dvPicker (DataSource: $Picker) { + combobox cbOneHop (Association: Bug1466.AreaPicker_District, + DataSource: database Bug1466.District where [Bug1466.District_Region = $currentObject/Bug1466.AreaPicker_Region], + CaptionAttribute: Name) + combobox cbInverted (Association: Bug1466.AreaPicker_District, + DataSource: database Bug1466.District where [Bug1466.District_Region/Bug1466.Region/Bug1466.AreaPicker_Region = $currentObject], + CaptionAttribute: Name) + combobox cbOwnAttr (Association: Bug1466.AreaPicker_District, + DataSource: database Bug1466.District where [Name = $currentObject/Label], + CaptionAttribute: Name) + listview lv (DataSource: database from Bug1466.District where [Bug1466.District_Region = $currentObject/Bug1466.AreaPicker_Region]) { + dynamictext t (Content: '{1}', ContentParams: ({1} = Name)) + } + } + } + } + } +}; diff --git a/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal.fail.mdl b/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal.fail.mdl new file mode 100644 index 0000000000..13d7712190 --- /dev/null +++ b/mdl-examples/bug-tests/1466-datasource-xpath-variable-traversal.fail.mdl @@ -0,0 +1,34 @@ +-- ============================================================================ +-- mendixlabs/mxcli#1466 — widget datasource XPath traversing off $currentObject +-- ============================================================================ +-- +-- NEGATIVE TEST (.fail.mdl): `mxcli check` MUST reject this file (MDL055). An +-- unexpected pass means the page-datasource arm of MDL055 has regressed. +-- +-- MDL055 (#831) only looked at microflow retrieves, so a widget's +-- `database … where […]` reaching two hops off `$currentObject` passed check +-- and exec, and the build failed: +-- +-- [error] [CE0161] "Error(s) in XPath constraint." at List view 'lv' +-- +-- Measured on fresh 11.13.0 and 11.14.0 projects: the two-hop form below is +-- CE0161 in a combobox, list view, data grid 2 and gallery. The ONE-hop form +-- the issue reported (`[Mod.A = $currentObject/Mod.B]`) builds at 0 errors in +-- each of them — see 1466-datasource-xpath-variable-traversal-ok.mdl. +-- ============================================================================ + +CREATE MODULE Bug1466F; +CREATE PERSISTENT ENTITY Bug1466F.Region (Name: String(100)); +CREATE PERSISTENT ENTITY Bug1466F.District (Name: String(100)); +CREATE PERSISTENT ENTITY Bug1466F.AreaPicker (Label: String(50)); +CREATE ASSOCIATION Bug1466F.AreaPicker_Region FROM Bug1466F.AreaPicker TO Bug1466F.Region TYPE Reference; + +CREATE PAGE Bug1466F.TwoHop ( + Params: ($Picker: Bug1466F.AreaPicker), Title: 'Area', Layout: Atlas_Core.Atlas_Default +) { + dataview dvPicker (DataSource: $Picker) { + listview lv (DataSource: database from Bug1466F.District where [Name = $currentObject/Bug1466F.AreaPicker_Region/Name]) { + dynamictext t (Content: '{1}', ContentParams: ({1} = Name)) + } + } +}; diff --git a/mdl/executor/validate_datasource_vartraversal_test.go b/mdl/executor/validate_datasource_vartraversal_test.go new file mode 100644 index 0000000000..8410a68445 --- /dev/null +++ b/mdl/executor/validate_datasource_vartraversal_test.go @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestDatasourceXPathVariableTraversal covers the page/widget-datasource arm of +// MDL055 (mendixlabs/mxcli#1466). The microflow-retrieve rule (#831) never saw a +// widget's `database … where […]`, so a constraint reaching two hops off +// `$currentObject` passed `mxcli check` and `exec` and the build failed CE0161. +// +// Measured on fresh 11.13.0 and 11.14.0 projects, inside a data view over the +// helper entity — the boundary is the same as for a retrieve: +// +// [Assoc = $currentObject/Mod.Assoc] 0 errors combobox, list view, data grid 2 +// [Mod.A/Mod.B/Mod.C = $currentObject] 0 errors combobox (the inverted form) +// [Name = $currentObject/Label] 0 errors combobox +// [Name = $currentObject/Mod.Assoc/Name] CE0161 combobox, list view, data grid 2, gallery +// +// The one-hop form the issue reported is valid; flagging it would refuse MDL +// that builds, so the controls below are as load-bearing as the flagged cases. +func TestDatasourceXPathVariableTraversal(t *testing.T) { + const twoHop = `[Name = $currentObject/MyModule.AreaPicker_Region/Name]` + const oneHop = `[MyModule.District_Region = $currentObject/MyModule.AreaPicker_Region]` + const inverted = `[MyModule.District_Region/MyModule.Region/MyModule.AreaPicker_Region = $currentObject]` + const ownAttr = `[Name = $currentObject/Label]` + + combobox := func(where string) string { + return `combobox cbDistrict (Association: MyModule.AreaPicker_District, + DataSource: database MyModule.District where ` + where + `, CaptionAttribute: Name)` + } + listview := func(where string) string { + return `listview lv (DataSource: database from MyModule.District where ` + where + `) { + dynamictext t (Content: '{1}', ContentParams: ({1} = Name)) + }` + } + datagrid := func(where string) string { + return `datagrid dg (DataSource: database from MyModule.District where ` + where + `) { + column (Attribute: Name, Caption: 'Name') + }` + } + gallery := func(where string) string { + return `gallery gal (DataSource: database from MyModule.District where ` + where + `) { + template { dynamictext t (Content: '{1}', ContentParams: ({1} = Name)) } + }` + } + + cases := []struct { + name string + widget string + flag bool + }{ + {"combobox two hops", combobox(twoHop), true}, + {"list view two hops", listview(twoHop), true}, + {"data grid 2 two hops", datagrid(twoHop), true}, + {"gallery two hops", gallery(twoHop), true}, + // Controls: each built at 0 errors. + {"combobox one hop — the reported form", combobox(oneHop), false}, + {"list view one hop", listview(oneHop), false}, + {"data grid 2 one hop", datagrid(oneHop), false}, + {"combobox inverted form", combobox(inverted), false}, + {"combobox own attribute", combobox(ownAttr), false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + src := `create page MyModule.P ( + Params: ($Picker: MyModule.AreaPicker), Title: 'Area', Layout: Atlas_Core.Atlas_Default +) { + dataview dvPicker (DataSource: $Picker) { + ` + c.widget + ` + } +};` + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse error: %v", errs[0]) + } + got := false + var msg string + for _, v := range ValidateWidgetProperties(prog, "") { + if v.RuleID == "MDL055" { + got = true + msg = v.Message + " / " + v.Suggestion + } + } + if got != c.flag { + t.Fatalf("MDL055 fired = %v, want %v\n message: %s", got, c.flag, msg) + } + if got && !strings.Contains(msg, "$currentObject/MyModule.AreaPicker_Region/Name") { + t.Errorf("message does not name the offending path: %s", msg) + } + }) + } +} diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 7b4f4be492..7e3852f6e1 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -942,19 +942,38 @@ var xpathVarTraversalRe = regexp.MustCompile(`\$(\w+)((?:/[A-Za-z_][\w.]*){2,})` // rejection rather than a writer fix: the constraint has to be restructured, and // only the author knows which of the two shapes they meant. func (v *microflowValidator) checkXPathVariableTraversal(variable, xpath string) { - for _, m := range xpathVarTraversalRe.FindAllStringSubmatch(xpath, -1) { - root, path := m[1], "$"+m[1]+m[2] - segs := strings.Split(strings.TrimPrefix(m[2], "/"), "/") - firstHop, leaf := segs[0], segs[len(segs)-1] + for _, t := range xpathVariableTraversals(xpath) { v.addViolation("MDL055", linter.SeverityError, fmt.Sprintf("retrieve '$%s' constraint traverses an association from a variable (`%s`), which Mendix XPath "+ "does not support (CE0161 \"Error(s) in XPath constraint\") — a constraint reaches at most one hop off a variable", - variable, path), + variable, t.path), fmt.Sprintf("Retrieve the associated object first, then constrain on its own attribute: "+ "`retrieve $Related from $%s/%s;` and use `[%s = $Related/%s]`. Or invert the constraint so the "+ "traversal starts at the entity being retrieved: `[%s/ = $%s]`. Both forms build clean.", - root, firstHop, leaf, leaf, firstHop, root)) + t.root, t.firstHop, t.leaf, t.leaf, t.firstHop, t.root)) + } +} + +// xpathVariableTraversal is one path of two or more hops off a $variable. +type xpathVariableTraversal struct { + path string // `$P/Mod.Assoc/Name`, as written + root string // `P` + firstHop string // `Mod.Assoc` + leaf string // `Name` +} + +// xpathVariableTraversals returns every path in an XPath constraint that reaches +// two or more hops off a $variable — the shape MDL055 refuses, in a microflow +// retrieve and in a widget's database datasource alike. +func xpathVariableTraversals(xpath string) []xpathVariableTraversal { + var out []xpathVariableTraversal + for _, m := range xpathVarTraversalRe.FindAllStringSubmatch(xpath, -1) { + segs := strings.Split(strings.TrimPrefix(m[2], "/"), "/") + out = append(out, xpathVariableTraversal{ + path: "$" + m[1] + m[2], root: m[1], firstHop: segs[0], leaf: segs[len(segs)-1], + }) } + return out } // checkSynchronizeIsNanoflowOnly rejects SYNCHRONIZE in a microflow. diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 3b6a50fc32..57ca3ccc91 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -276,6 +276,7 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc // as its text (ako/mxcli#969). out = append(out, validateQuotedTemplateParams(w, locationPrefix)...) out = append(out, validateDatasourceXPathAssociationEmpty(w, locationPrefix)...) + out = append(out, validateDatasourceXPathVariableTraversal(w, locationPrefix)...) out = append(out, validateComboBoxAssociation(w, locationPrefix)...) // #631: inputs inside a list view that will be written read-only. out = append(out, validateListViewEditableInputs(w, registry, locationPrefix)...) @@ -366,6 +367,38 @@ func validateDatasourceXPathAssociationEmpty(w *ast.WidgetV3, locationPrefix str return out } +// validateDatasourceXPathVariableTraversal flags a widget database datasource +// whose `where` reaches two or more hops off a variable +// (`[Name = $currentObject/Mod.Assoc/Name]`) — the page-datasource counterpart +// of the microflow-retrieve MDL055 check (mendixlabs/mxcli#1466). +// +// Measured on fresh 11.13.0 and 11.14.0 projects: the two-hop form is CE0161 in +// a combobox, list view, data grid 2 and gallery, while one hop +// (`[Mod.A = $currentObject/Mod.B]`), the context object's own attribute and +// the inverted form (`[Mod.A/Mod.E/Mod.B = $currentObject]`) all build at 0 +// errors — the same boundary as a retrieve, so the same regex. +func validateDatasourceXPathVariableTraversal(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + ds := w.GetDataSource() + if ds == nil || ds.Where == "" { + return nil + } + var out []linter.Violation + for _, t := range xpathVariableTraversals(ds.Where) { + out = append(out, linter.Violation{ + RuleID: "MDL055", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: widget `%s` datasource constraint traverses an association from a variable (`%s`), which Mendix XPath does not support (CE0161 \"Error(s) in XPath constraint\") — a constraint reaches at most one hop off a variable", + locationPrefix, w.Name, t.path), + Suggestion: fmt.Sprintf( + "Invert the constraint so the traversal starts at the listed entity: `[//%s = $%s]`, "+ + "or compare one hop: `[ = $%s/%s]`. Both build clean.", + t.firstHop, t.root, t.root, t.firstHop), + }) + } + return out +} + // comboBoxOptionsDataSourceKey is the ComboBox schema property holding the // association mode's option list — the named spelling of what the generic // `datasource:` clause fills. From fc1114f463d3d2737591b239b259cfc3dcda5716 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:11:17 +0000 Subject: [PATCH 06/23] docs(skills): write the legacy-grid alter example as a full statement (mendixlabs/mxcli#1459) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conformance gate parses every MDL block in a skill; the bare `drop column14, …` fragment was one finding over the file's allowlist. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .claude/skills/mendix/alter-page/SKILL.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index b76c6afa97..a3f7224558 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -404,8 +404,11 @@ The older dotted form `gridName.columnName` still works; it matches a name mxcli A **legacy (Dojo) data grid** (`Forms$DataGrid`, from an upgraded app) cannot be created in MDL, so `describe page` prints it as comments — but it lists every column, search field and control-bar button by its *stored* name. Those are addressed by that bare name, like any widget: ```sql -drop column14, searchField3 -- one column and its search field -set (caption: 'Note') on column2 +mdl 1; +alter page MyMod.Order_Overview { + drop column14, searchField3; -- one column and its search field + set (caption: 'Note') on column2 +}; ``` Only that item is removed or changed; the rest of the grid is left as stored. To restructure the grid itself, replace it with a DataGrid 2. From 1641c40247bc10a5804cca78559cece8dfaf8653 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:05:56 +0000 Subject: [PATCH 07/23] fix(pages): describe and write role-based widget visibility (mendixlabs/mxcli#1460) ConditionalVisibilitySettings.ModuleRoles was never read by describe, had no MDL spelling, was written empty, and was dropped by ALTER set Visible. Add `Visible: roles (Mod.A, ...)` (combinable with an expression/attribute condition) on widgets, tab pages and navigation-list items; check --references resolves the roles. Tests at describe/visitor/builder/writer/mutator layers; each fails with the drop when its fix is stubbed out. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...0-13-visible-for-module-roles-dropped.json | 1 + .claude/skills/mendix/create-page/SKILL.md | 16 ++ docs-site/src/language/pages.md | 20 +++ .../1460-visible-for-module-roles.mdl | 43 +++++ .../modelsdk/widget_conditional_test.go | 59 +++++++ mdl/backend/modelsdk/widget_write.go | 8 + mdl/backend/pagemutator/mutator.go | 41 ++++- mdl/backend/pagemutator/mutator_test.go | 47 ++++++ .../check_grant_references_pedapp_test.go | 8 + mdl/executor/cmd_alter_page.go | 2 +- mdl/executor/cmd_pages_builder_v3.go | 6 + mdl/executor/cmd_pages_builder_v3_layout.go | 1 + mdl/executor/cmd_pages_builder_v3_widgets.go | 12 ++ .../cmd_pages_builder_visible_when.go | 50 ++++++ mdl/executor/cmd_pages_describe.go | 5 +- mdl/executor/cmd_pages_describe_output.go | 25 +++ mdl/executor/cmd_pages_describe_parse.go | 11 ++ mdl/executor/cmd_pages_visible_roles_test.go | 155 ++++++++++++++++++ mdl/executor/validate_grant_refs.go | 18 ++ mdl/executor/validate_widgets.go | 2 +- mdl/executor/widget_engine.go | 2 +- mdl/grammar/domains/MDLPage.g4 | 12 ++ mdl/visitor/visitor_page_v3.go | 11 ++ sdk/pages/pages_widgets.go | 8 +- sdk/pages/pages_widgets_action.go | 4 + 25 files changed, 557 insertions(+), 10 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-13-visible-for-module-roles-dropped.json create mode 100644 mdl-examples/bug-tests/1460-visible-for-module-roles.mdl create mode 100644 mdl/executor/cmd_pages_visible_roles_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-visible-for-module-roles-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-visible-for-module-roles-dropped.json new file mode 100644 index 0000000000..e25658b7d3 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-visible-for-module-roles-dropped.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-13", "symptom": "Role-based widget visibility (ConditionalVisibilitySettings.ModuleRoles) is not printed by DESCRIBE, so a `create or replace` from describe output silently drops it — the widget becomes visible to every role, with mx check clean", "cause": "Four gaps on one property: `extractConditionalSettings` read Expression and Attribute/Conditions and never ModuleRoles; MDL had no spelling for it; `conditionalVisibilityToGen` always wrote the list empty (and `pages.ConditionalVisibilitySettings.ModuleRoles` was an unused `[]model.ID` though the property is BY_NAME); ALTER `set Visible` rebuilt the settings node from scratch, dropping stored roles. Navigation-list items read and wrote no visibility at all", "file": "`mdl/executor/cmd_pages_describe_parse.go` (`extractConditionalSettings`, `parseNavigationListItems`), `mdl/executor/cmd_pages_builder_visible_when.go` (`applyVisibleRoles`/`withVisibleRoles`), `mdl/backend/modelsdk/widget_write.go` (`conditionalVisibilityToGen`, `navListItemToGen`), `mdl/backend/pagemutator/mutator.go` (`setWidgetConditionalSettingMut`), `mdl/executor/validate_grant_refs.go`, `MDLPage.g4` (`Visible: roles (…)`)", "insight": "The roles are a SEPARATE setting from the condition, stored on the same element: TestApp's Studio Pro-authored WorkflowCommons snippets carry Expression + ModuleRoles [1, \"WorkflowCommons.Administrator\"] on one node, and Attribute + Conditions + ModuleRoles on another, and roles alone (Expression \"\") on a third — so the MDL is a second `Visible:` line, not a variant of the first, and every path that rebuilds the node (builder tail, ALTER SET, tab page, nav item) must carry it. A BSON scan of mprcontents for non-empty ModuleRoles settled the shape in minutes; the roundtrip allowlist's 'a module-role visibility drifts' note pointed at the sample. mx 11.13 resolves the names: a dangling role is CE1613 \"The selected module role … no longer exists\" at the widget (positive control), valid roles add no error (1002 = control). Revert controls: stubbing the describe read, the writer and the builder each fails its test with the reported drop", "refs": ["mendixlabs/mxcli#1460"]} diff --git a/.claude/skills/mendix/create-page/SKILL.md b/.claude/skills/mendix/create-page/SKILL.md index a196908fc0..9141df0921 100644 --- a/.claude/skills/mendix/create-page/SKILL.md +++ b/.claude/skills/mendix/create-page/SKILL.md @@ -335,6 +335,22 @@ container cntRunning (visible: Status in (Running, empty)) { ... } textbox txtPassword (label: 'Password', attribute: Password, visible: IsLocalUser in (true)) ``` +**Visible for module roles** (Studio Pro's role-based visibility) is a separate +setting — a second `visible:` line that combines with either condition above. It +works on widgets, tab pages and navigation-list items; `check --references` +refuses a role that does not exist: + +```sql +mdl 1; +create page MyModule.Request_Review (title: 'Review', layout: Atlas_Core.Atlas_Default) { + container cntAdmin (visible: roles (MyModule.Administrator)) { + dynamictext txtAdmin (content: 'Administrators only') + } + actionbutton btnClose (caption: 'Close', action: close page, + visible: false, visible: roles (MyModule.Manager)) +}; +``` + > **`visible:`/`editable:` is a Mendix *expression*, not XPath** — a different > function set from a datasource `where [ ... ]` clause: diff --git a/docs-site/src/language/pages.md b/docs-site/src/language/pages.md index 446f051472..db83a9221d 100644 --- a/docs-site/src/language/pages.md +++ b/docs-site/src/language/pages.md @@ -116,6 +116,26 @@ mxcli writes one condition per value of the attribute, as Studio Pro does, so a value not listed hides the widget. `describe page` emits this form for widgets set up that way in Studio Pro. +Studio Pro's **"Visible for module roles"** restricts a widget to users holding +one of the named module roles. It is a separate setting, so it combines with an +expression or attribute condition — the widget then carries two `Visible:` lines: + +```sql +mdl 1; +create page MyModule.Request_Review (Title: 'Review', Layout: Atlas_Core.Atlas_Default) { + container cntAdmin (Visible: roles (MyModule.Administrator)) { + dynamictext txtAdmin (Content: 'Administrators only') + } + actionbutton btnClose (Caption: 'Close', Action: close page, + Visible: false, Visible: roles (MyModule.Administrator, MyModule.Editor)) +}; +``` + +It is accepted on widgets, tab pages and navigation-list items. `mxcli check +--references` refuses a role the project (or the script, above the statement) +does not define. `alter page … set Visible` changes only the condition and keeps +the stored roles. + ### Conditional Editability Input widgets can be conditionally editable: diff --git a/mdl-examples/bug-tests/1460-visible-for-module-roles.mdl b/mdl-examples/bug-tests/1460-visible-for-module-roles.mdl new file mode 100644 index 0000000000..2010dc0427 --- /dev/null +++ b/mdl-examples/bug-tests/1460-visible-for-module-roles.mdl @@ -0,0 +1,43 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1460 — role-based widget visibility dropped by describe +-- ============================================================================ +-- +-- Symptom (before fix): "Role-based widget visibility +-- (ConditionalVisibilitySettings.ModuleRoles) is not printed by DESCRIBE", so a +-- `create or replace` from describe output removed it with no warning — the +-- widget became visible to every role. MDL had no way to write it either. +-- +-- Root cause: describe read Expression and Attribute/Conditions from the +-- settings element and never ModuleRoles; the writer always emitted the list +-- empty; ALTER `set Visible` rebuilt the element without it. +-- +-- After fix: `Visible: roles (Mod.A, …)` is a second `Visible:` line, beside an +-- expression or attribute condition, on widgets, tab pages and +-- navigation-list items. Stored as ModuleRoles [1, "Mod.A", …] (by-name, the +-- shape of TestApp's Studio Pro-authored WorkflowCommons snippets). +-- +-- Verify (Mendix 11.13, testdata/pedapp copy): +-- mxcli check 1460-visible-for-module-roles.mdl -p app.mpr --references +-- mxcli exec 1460-visible-for-module-roles.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe page MyFirstModule.Bug1460Roles" -- roles printed +-- re-exec the describe output -- "Unchanged" +-- mxcli docker check -p app.mpr -- no new errors (a dangling role is CE1613) +-- ============================================================================ + +create module role MyFirstModule.Bug1460Admin; +create module role MyFirstModule.Bug1460Viewer; + +create page MyFirstModule.Bug1460Roles (Title: 'Roles', Layout: Atlas_Core.Atlas_Default) { + container cAdminOnly (Visible: roles (MyFirstModule.Bug1460Admin)) { + dynamictext t1 (Content: 'admins only') + } + container cBoth (Visible: false, Visible: roles (MyFirstModule.Bug1460Admin, MyFirstModule.Bug1460Viewer)) { + dynamictext t2 (Content: 'never') + } + navigationlist nav { + item (Action: close page, Visible: roles (MyFirstModule.Bug1460Viewer)) { + dynamictext t3 (Content: 'viewers') + } + } +}; diff --git a/mdl/backend/modelsdk/widget_conditional_test.go b/mdl/backend/modelsdk/widget_conditional_test.go index 9728c059ab..9b8d7b0e3a 100644 --- a/mdl/backend/modelsdk/widget_conditional_test.go +++ b/mdl/backend/modelsdk/widget_conditional_test.go @@ -80,3 +80,62 @@ func TestWidgetConditionalVisibility_NullWhenUnset(t *testing.T) { t.Errorf("expected null ConditionalVisibilitySettings when unset, got %T", v) } } + +// A navigation-list item's visibility is written like a widget's (#1460); with +// none it stays the null Studio Pro stores (ako/mxcli#950). +func TestNavigationListItemConditionalVisibility_ModuleRoles(t *testing.T) { + item := &pages.NavigationListItem{Action: &pages.NoClientAction{}, + ConditionalVisibility: &pages.ConditionalVisibilitySettings{ + BaseElement: model.BaseElement{TypeName: "Forms$ConditionalVisibilitySettings"}, + ModuleRoles: []string{"M.Admin"}, + }} + el, err := navListItemToGen(item) + if err != nil { + t.Fatal(err) + } + cvs, ok := encodeToMap(t, el)["ConditionalVisibilitySettings"].(map[string]any) + if !ok { + t.Fatal("item ConditionalVisibilitySettings not written — the role restriction was dropped") + } + if roles, _ := cvs["ModuleRoles"].(bsonv1.A); len(roles) != 2 || roles[1] != "M.Admin" { + t.Errorf("ModuleRoles = %v, want [1 M.Admin]", cvs["ModuleRoles"]) + } + el, err = navListItemToGen(&pages.NavigationListItem{Action: &pages.NoClientAction{}}) + if err != nil { + t.Fatal(err) + } + if v := encodeToMap(t, el)["ConditionalVisibilitySettings"]; v != nil { + t.Errorf("an item without visibility must store null, got %v", v) + } +} + +// mendixlabs/mxcli#1460: role-based visibility is ConditionalVisibilitySettings. +// ModuleRoles, a by-name list — [1, "WorkflowCommons.Administrator"] on TestApp's +// Studio Pro-authored WorkflowCommons snippets, beside an expression or alone. +// The writer emitted the list empty whatever the model held. +func TestWidgetConditionalVisibility_ModuleRoles(t *testing.T) { + ctn := &pages.Container{} + ctn.Name = "ctn" + ctn.ConditionalVisibility = &pages.ConditionalVisibilitySettings{ + BaseElement: model.BaseElement{TypeName: "Forms$ConditionalVisibilitySettings"}, + Expression: "$currentObject/Name != ''", + ModuleRoles: []string{"M.Admin", "M.User"}, + } + cvs, ok := docGet(encodeWidget(t, ctn), "ConditionalVisibilitySettings").(bsonv1.D) + if !ok { + t.Fatal("ConditionalVisibilitySettings not serialized") + } + roles, _ := docGet(cvs, "ModuleRoles").(bsonv1.A) + want := bsonv1.A{int32(1), "M.Admin", "M.User"} + if len(roles) != len(want) { + t.Fatalf("ModuleRoles = %v, want %v", roles, want) + } + for i := range want { + if roles[i] != want[i] { + t.Errorf("ModuleRoles[%d] = %v (%T), want %v", i, roles[i], roles[i], want[i]) + } + } + if got := docGet(cvs, "Expression"); got != "$currentObject/Name != ''" { + t.Errorf("Expression = %v — the role list must not displace it", got) + } +} diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index ddee9a926a..93200da186 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -1009,6 +1009,9 @@ func navListItemToGen(item *pages.NavigationListItem) (element.Element, error) { // on 11.14.0 with two named items) — the CE7247 / CE0495 an older note // blamed on its absence is not reproduced. g.SetAppearance(newAppearance(item.Class, item.Style, item.DynamicClasses, nil)) + if item.ConditionalVisibility != nil { + g.SetConditionalVisibilitySettings(conditionalVisibilityToGen(item.ConditionalVisibility)) + } act, err := clientActionToGen(item.Action) if err != nil { return nil, err @@ -1153,6 +1156,11 @@ func conditionalVisibilityToGen(cvs *pages.ConditionalVisibilitySettings) elemen assignID(g) g.SetExpression(cvs.Expression) g.SetIgnoreSecurity(false) + // "Visible for module roles" (mendixlabs/mxcli#1460): by-name references, + // independent of the expression/attribute condition beside them. + if len(cvs.ModuleRoles) > 0 { + g.SetModuleRolesQualifiedNames(cvs.ModuleRoles) + } // "Visible: based on attribute value": the attribute plus one condition per // value it can hold (ako/mxcli attribute-condition visibility). if cvs.Attribute != "" { diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 20dfaca8fc..19fc521281 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -2850,6 +2850,15 @@ func coercePopupDimension(prop string, value any) (int64, error) { // marker-3 Conditions, plus IgnoreSecurity/ModuleRoles for visibility). Returns // false when the widget has no such slot (e.g. editability on a non-input widget). func setWidgetConditionalSettingMut(widget bson.D, field, typeName, expression string, withSecurity bool) bool { + // "Visible for module roles" lives on the visibility node beside the + // condition; rebuilding the node for a new condition must not drop it + // (mendixlabs/mxcli#1460). Carried verbatim, list marker included. + moduleRoles := any(bson.A{int32(3)}) + if withSecurity { + if stored := storedVisibleRoles(widget); stored != nil { + moduleRoles = stored + } + } doc := bson.D{ {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, {Key: "$Type", Value: typeName}, @@ -2867,13 +2876,32 @@ func setWidgetConditionalSettingMut(widget bson.D, field, typeName, expression s if withSecurity { doc = append(doc, bson.E{Key: "IgnoreSecurity", Value: false}, - bson.E{Key: "ModuleRoles", Value: bson.A{int32(3)}}, + bson.E{Key: "ModuleRoles", Value: moduleRoles}, ) } doc = append(doc, bson.E{Key: "SourceVariable", Value: nil}) return bsonnav.DSet(widget, field, doc) } +// storedVisibleRoles is the widget's stored ConditionalVisibilitySettings. +// ModuleRoles list when it names at least one role, else nil. +func storedVisibleRoles(widget bson.D) bson.A { + cvs, ok := bsonnav.DGet(widget, "ConditionalVisibilitySettings").(bson.D) + if !ok { + return nil + } + roles, ok := bsonnav.DGet(cvs, "ModuleRoles").(bson.A) + if !ok { + return nil + } + for _, r := range roles { + if _, isName := r.(string); isName { + return roles + } + } + return nil +} + // setWidgetEditableMut writes `set Editable = …`. Two widget families store two // different things under the same key, so the stored value's type decides what // the statement may say: @@ -3000,9 +3028,14 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { // Pro silently dropped it). The `[expr]` bracket form arrives as VisibleIf. expr, hasSetting := pages.StaticVisibleExpression(value) if !hasSetting { - // `Visible = true` (default-visible): clear any conditional-visibility node. - bsonnav.DSet(widget, "ConditionalVisibilitySettings", nil) - return nil + // `Visible = true` (default-visible): clear any conditional-visibility + // node — unless it carries module roles, a separate setting that + // Studio Pro stores on the same node (#1460): then only the + // condition is cleared. + if storedVisibleRoles(widget) == nil { + bsonnav.DSet(widget, "ConditionalVisibilitySettings", nil) + return nil + } } if !setWidgetConditionalSettingMut(widget, "ConditionalVisibilitySettings", "Forms$ConditionalVisibilitySettings", expr, true) { diff --git a/mdl/backend/pagemutator/mutator_test.go b/mdl/backend/pagemutator/mutator_test.go index d63667de8a..9e96bc7868 100644 --- a/mdl/backend/pagemutator/mutator_test.go +++ b/mdl/backend/pagemutator/mutator_test.go @@ -1408,6 +1408,53 @@ func TestSetWidgetProperty_VisibleIf(t *testing.T) { } } +// mendixlabs/mxcli#1460: Studio Pro stores "Visible for module roles" on the +// same node as the condition, so SET Visible rebuilt the node and dropped the +// role restriction. The roles are a separate setting and must survive a SET of +// the condition — including `Visible = true`, which clears only the condition. +func TestSetWidgetProperty_VisibleKeepsModuleRoles(t *testing.T) { + for _, tc := range []struct { + prop string + value any + wantExpr string + }{ + {"VisibleIf", "$currentObject/Name != ''", "$currentObject/Name != ''"}, + {"Visible", false, "false"}, + {"Visible", true, ""}, + } { + w := bson.D{ + {Key: "$Type", Value: "Forms$DivContainer"}, + {Key: "Name", Value: "ctn"}, + {Key: "ConditionalVisibilitySettings", Value: bson.D{ + {Key: "$ID", Value: "x"}, + {Key: "$Type", Value: "Forms$ConditionalVisibilitySettings"}, + {Key: "Attribute", Value: ""}, + {Key: "Conditions", Value: bson.A{int32(2)}}, + {Key: "Expression", Value: "old"}, + {Key: "IgnoreSecurity", Value: false}, + {Key: "ModuleRoles", Value: bson.A{int32(1), "M.Admin"}}, + {Key: "SourceVariable", Value: nil}, + }}, + } + rawData := makeRawPage(w) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + if err := m.SetWidgetProperty("ctn", tc.prop, tc.value); err != nil { + t.Fatalf("%s = %v: %v", tc.prop, tc.value, err) + } + cvs, ok := bsonnav.DGet(findBsonWidget(rawData, "ctn").widget, "ConditionalVisibilitySettings").(bson.D) + if !ok { + t.Fatalf("%s = %v: settings node removed — the role restriction was dropped", tc.prop, tc.value) + } + if got := bsonnav.DGetString(cvs, "Expression"); got != tc.wantExpr { + t.Errorf("%s = %v: Expression = %q, want %q", tc.prop, tc.value, got, tc.wantExpr) + } + roles, _ := bsonnav.DGet(cvs, "ModuleRoles").(bson.A) + if len(roles) != 2 || roles[1] != "M.Admin" { + t.Errorf("%s = %v: ModuleRoles = %v, want [1 M.Admin]", tc.prop, tc.value, roles) + } + } +} + func TestSetWidgetProperty_EditableIf_Unsupported(t *testing.T) { // A container has no ConditionalEditabilitySettings slot → reject. w := bson.D{ diff --git a/mdl/executor/check_grant_references_pedapp_test.go b/mdl/executor/check_grant_references_pedapp_test.go index e07ca41c9f..23fcde3a50 100644 --- a/mdl/executor/check_grant_references_pedapp_test.go +++ b/mdl/executor/check_grant_references_pedapp_test.go @@ -113,6 +113,10 @@ func TestCheckGrantReferences_UnknownNameIsRefused(t *testing.T) { "create module AutoRoleYes;\ncreate nanoflow AutoRoleYes.N () begin end;\n" + "grant execute on nanoflow AutoRoleYes.N to AutoRoleYes.User;", []string{"module role AutoRoleYes.User does not exist"}}, + {"page widget visible to a module role that does not exist (exec writes it, CE1613; #1460)", + "create page MyFirstModule.RoleVisProbe (Title: 'x', Layout: Atlas_Core.Atlas_Default) {\n" + + " container c1 (Visible: roles (MyFirstModule.NopeRole)) { dynamictext t1 (Content: 'x') }\n};", + []string{"module role MyFirstModule.NopeRole does not exist", "container \"c1\": Visible: roles"}}, {"grant to a role the script dropped above it", "drop module role Administration.User;\ngrant read * on entity Administration.Account to Administration.User;", []string{"module role Administration.User does not exist"}}, @@ -176,6 +180,10 @@ func TestCheckGrantReferences_ResolvableNamesPass(t *testing.T) { {"grant to a .User role the script creates with the module (doctype-tests/02b)", "create module AutoRoleYes;\ncreate module role AutoRoleYes.User;\ncreate nanoflow AutoRoleYes.N () begin end;\n" + "grant execute on nanoflow AutoRoleYes.N to AutoRoleYes.User;"}, + {"page widget visible to a stored role and one the script creates above (#1460)", + "create module role MyFirstModule.Auditor;\n" + + "create page MyFirstModule.RoleVisProbe (Title: 'x', Layout: Atlas_Core.Atlas_Default) {\n" + + " container c1 (Visible: roles (MyFirstModule.User, MyFirstModule.Auditor)) { dynamictext t1 (Content: 'x') }\n};"}, {"user role and demo user the script creates above", "create module role MyFirstModule.Auditor;\ncreate user role Auditor ( ModuleRoles: (MyFirstModule.Auditor, System.User) );\n" + "create demo user 'auditor' ( Password: 'Probe!23456789', UserRoles: (Auditor) );"}, diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index d94fa34757..c546dcc190 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -765,7 +765,7 @@ func replacePluggableKeepingUnstated(ctx *ExecContext, mutator backend.PageMutat // replacement differs from the baseline" as "the statement states it", so one // the statement leaves out must be left out of the baseline too, or the // replacement's default would overwrite the stored value. -var replaceBaselineSystemProps = []string{"Editable", "EditableIf", "Visible", "VisibleIf", "VisibleWhen"} +var replaceBaselineSystemProps = []string{"Editable", "EditableIf", "Visible", "VisibleIf", "VisibleWhen", "VisibleRoles"} // replaceBaseline is the stored widget's description with the visibility and // editability properties the statement does not state removed. diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 0e1e83ae20..07a57466ff 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -534,6 +534,9 @@ func (pb *pageBuilder) buildWidgetV3(w *ast.WidgetV3) (pages.Widget, error) { if err := pb.applyVisibleWhen(widget, w); err != nil { return nil, err } + if err := applyVisibleRoles(widget, w); err != nil { + return nil, err + } return widget, nil } @@ -563,6 +566,9 @@ func (pb *pageBuilder) buildPluggable(def *WidgetDefinition, w *ast.WidgetV3) (p if err := pb.applyVisibleWhen(widget, w); err != nil { return nil, err } + if err := applyVisibleRoles(widget, w); err != nil { + return nil, err + } return widget, nil } diff --git a/mdl/executor/cmd_pages_builder_v3_layout.go b/mdl/executor/cmd_pages_builder_v3_layout.go index d3fded454e..3e10331876 100644 --- a/mdl/executor/cmd_pages_builder_v3_layout.go +++ b/mdl/executor/cmd_pages_builder_v3_layout.go @@ -350,6 +350,7 @@ func (pb *pageBuilder) buildTabPageV3(w *ast.WidgetV3) (*pages.TabPage, error) { } tp.ConditionalVisibility = cvs } + tp.ConditionalVisibility = withVisibleRoles(tp.ConditionalVisibility, w) // Build child widgets for _, child := range w.Children { diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index 26e2839484..350ee59851 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -1324,6 +1324,18 @@ func (pb *pageBuilder) buildNavigationListItemV3(w *ast.WidgetV3) (*pages.Naviga item.Style = w.GetStyle() item.DynamicClasses = w.GetDynamicClasses() + // The item's visibility, as a widget's: expression or attribute condition, + // and module roles beside either (#1460). + item.ConditionalVisibility = expressionVisibility(w) + if vw, ok := w.Properties["VisibleWhen"].(*ast.VisibleWhenV3); ok && vw != nil { + cvs, err := pb.visibleWhenSettings(w, vw) + if err != nil { + return nil, err + } + item.ConditionalVisibility = cvs + } + item.ConditionalVisibility = withVisibleRoles(item.ConditionalVisibility, w) + // Set caption from Caption property if caption := w.GetCaption(); caption != "" { item.Caption = &model.Text{ diff --git a/mdl/executor/cmd_pages_builder_visible_when.go b/mdl/executor/cmd_pages_builder_visible_when.go index 53eeada6c6..fbeabf3cba 100644 --- a/mdl/executor/cmd_pages_builder_visible_when.go +++ b/mdl/executor/cmd_pages_builder_visible_when.go @@ -136,6 +136,56 @@ func (pb *pageBuilder) visibleWhenSettings(w *ast.WidgetV3, vw *ast.VisibleWhenV }, nil } +// applyVisibleRoles writes `Visible: roles (Mod.A, …)` — Studio Pro's "Visible +// for module roles" — onto the widget's settings element, creating one when no +// expression or attribute condition did. Studio Pro keeps the roles beside the +// condition on the same element (TestApp WorkflowCommons), so they never +// replace it. Runs after applyConditionalSettings and applyVisibleWhen. +func applyVisibleRoles(widget pages.Widget, w *ast.WidgetV3) error { + if len(widgetVisibleRoles(w)) == 0 { + return nil + } + type baseWidgetGetter interface { + GetBaseWidget() *pages.BaseWidget + } + bwg, ok := widget.(baseWidgetGetter) + if !ok { + return mdlerrors.NewValidationf("%s %s: `Visible: roles (…)` is not supported on this widget", w.Type, w.Name) + } + bw := bwg.GetBaseWidget() + bw.ConditionalVisibility = withVisibleRoles(bw.ConditionalVisibility, w) + return nil +} + +// withVisibleRoles returns cvs carrying the statement's `Visible: roles (…)`, +// a fresh settings element when cvs is nil, and cvs unchanged when the +// statement names no roles. +func withVisibleRoles(cvs *pages.ConditionalVisibilitySettings, w *ast.WidgetV3) *pages.ConditionalVisibilitySettings { + roles := widgetVisibleRoles(w) + if len(roles) == 0 { + return cvs + } + if cvs == nil { + cvs = &pages.ConditionalVisibilitySettings{ + BaseElement: model.BaseElement{ + ID: model.ID(types.GenerateID()), + TypeName: "Forms$ConditionalVisibilitySettings", + }, + } + } + cvs.ModuleRoles = roles + return cvs +} + +// widgetVisibleRoles is the statement's `Visible: roles (…)` list, if any. +func widgetVisibleRoles(w *ast.WidgetV3) []string { + if w == nil { + return nil + } + roles, _ := w.Properties["VisibleRoles"].([]string) + return roles +} + func indexFold(values []string, v string) int { for i, x := range values { if strings.EqualFold(x, v) { diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index f39888d840..e690055d37 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -842,7 +842,10 @@ type rawWidget struct { // values that SHOW the widget, as MDL spells them (`empty` for "(empty)"). VisibleAttr string VisibleValues []string - EditableIf string // Expression from ConditionalEditabilitySettings + // "Visible for module roles": qualified role names, independent of the + // expression/attribute condition (mendixlabs/mxcli#1460). + VisibleRoles []string + EditableIf string // Expression from ConditionalEditabilitySettings // Design properties from Appearance DesignProperties []rawDesignProp // Explicit widget properties (for generic PLUGGABLEWIDGET output) diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index a394329614..0916899d8b 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -194,6 +194,9 @@ func appendAppearanceProps(ctx *ExecContext, props []string, w rawWidget) []stri if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } + if prop := visibleRolesProp(w); prop != "" { + props = append(props, prop) + } if w.EditableIf != "" { props = append(props, widgetConditionMDL("Editable", w.EditableIf)) } @@ -397,6 +400,9 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } + if prop := visibleRolesProp(w); prop != "" { + props = append(props, prop) + } if len(w.Children) > 0 { formatWidgetProps(ctx.Output, prefix, header, props, " {\n") for _, child := range w.Children { @@ -984,6 +990,15 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if child.DynamicClasses != "" { props = append(props, fmt.Sprintf("DynamicClasses: %s", child.DynamicClasses)) // an expression, printed as-is } + if child.VisibleIf != "" { + props = append(props, widgetConditionMDL("Visible", child.VisibleIf)) + } + if prop := visibleWhenProp(child); prop != "" { + props = append(props, prop) + } + if prop := visibleRolesProp(child); prop != "" { + props = append(props, prop) + } formatWidgetProps(ctx.Output, prefix+" ", itemHeader, props, " {\n") for _, cw := range child.Children { outputWidgetMDLV3(ctx, cw, indent+2) @@ -2170,6 +2185,16 @@ func visibleWhenProp(w rawWidget) string { return fmt.Sprintf("Visible: %s in (%s)", attr, strings.Join(vals, ", ")) } +// visibleRolesProp renders "Visible for module roles" as `Visible: roles +// (Mod.A, Mod.B)`. It is a second `Visible:` line beside an expression or +// attribute condition, as Studio Pro stores both on one settings element. +func visibleRolesProp(w rawWidget) string { + if len(w.VisibleRoles) == 0 { + return "" + } + return fmt.Sprintf("Visible: roles (%s)", strings.Join(w.VisibleRoles, ", ")) +} + // layoutGridWidthMDL spells a stored layout-grid column weight: 1..12 is // printed as the number, -2 (auto-fit content) as AutoFit, and anything else — // -1, or absent on a model older than the property — as AutoFill. diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 3caf190e20..2bcd021f07 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -59,6 +59,14 @@ func extractConditionalSettings(ctx *ExecContext, widget *rawWidget, w map[strin widget.VisibleValues = append(widget.VisibleValues, v) } } + // "Visible for module roles": by-name Module.Role references, stored + // beside (not instead of) the condition above. Never read, so describe + // → exec removed the restriction (mendixlabs/mxcli#1460). + for _, r := range getBsonArrayElements(cvs["ModuleRoles"]) { + if name, ok := r.(string); ok && name != "" { + widget.VisibleRoles = append(widget.VisibleRoles, name) + } + } } if ces, ok := w["ConditionalEditabilitySettings"].(map[string]any); ok && ces != nil { if expr, ok := ces["Expression"].(string); ok && expr != "" { @@ -896,6 +904,9 @@ func parseNavigationListItems(ctx *ExecContext, w map[string]any) []rawWidget { rw.Style, _ = appearance["Style"].(string) rw.DynamicClasses, _ = appearance["DynamicClasses"].(string) } + // The item's own visibility — 13 of the report's 97 role-locked + // widgets were items of one navigation list (#1460). + extractConditionalSettings(ctx, &rw, itemMap) result = append(result, rw) } diff --git a/mdl/executor/cmd_pages_visible_roles_test.go b/mdl/executor/cmd_pages_visible_roles_test.go new file mode 100644 index 0000000000..3e118ccc85 --- /dev/null +++ b/mdl/executor/cmd_pages_visible_roles_test.go @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "context" + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1460: "Role-based widget visibility +// (ConditionalVisibilitySettings.ModuleRoles) is not printed by DESCRIBE", so a +// `create or replace` from describe output silently dropped it. The stored +// shape is TestApp's WorkflowCommons.Snip_WorkflowActivityRecord_Header: an +// expression AND a role list on one settings element. +func roleVisibleStored() map[string]any { + return map[string]any{ + "$Type": "Forms$DivContainer", + "Name": "c1", + "ConditionalVisibilitySettings": map[string]any{ + "$Type": "Forms$ConditionalVisibilitySettings", + "Attribute": "", + "Conditions": []any{int32(2)}, + "Expression": "$currentObject/Name != empty", + "ModuleRoles": []any{int32(1), "WorkflowCommons.Administrator", "WorkflowCommons.User"}, + }, + "Widgets": []any{int32(2)}, + } +} + +func TestDescribe_VisibleRoles(t *testing.T) { + var buf bytes.Buffer + ctx := (&Executor{}).newExecContext(context.Background()) + ctx.Output = &buf + raw := parseRawWidget(ctx, roleVisibleStored()) + outputWidgetMDLV3(ctx, raw[0], 1) + got := buf.String() + if !strings.Contains(got, "Visible: roles (WorkflowCommons.Administrator, WorkflowCommons.User)") { + t.Errorf("describe output lacks the role visibility:\n%s", got) + } + if !strings.Contains(got, "Visible: $currentObject/Name != empty") { + t.Errorf("describe output lost the expression beside the roles:\n%s", got) + } + + // The output parses back to both settings. + prog, errs := visitor.Build("create page M.P (Title: 'x', Layout: A.L) {\n" + got + "}\n") + if len(errs) > 0 { + t.Fatalf("describe output does not parse: %v\n%s", errs, got) + } + w := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0] + if roles, _ := w.Properties["VisibleRoles"].([]string); !reflect.DeepEqual(roles, + []string{"WorkflowCommons.Administrator", "WorkflowCommons.User"}) { + t.Errorf("VisibleRoles = %#v", w.Properties["VisibleRoles"]) + } + if w.GetStringProp("VisibleIf") != "$currentObject/Name != empty" { + t.Errorf("VisibleIf = %q", w.GetStringProp("VisibleIf")) + } +} + +// A navigation-list item carries its own ConditionalVisibilitySettings — 13 of +// the 97 role-locked widgets in the report were items of one navigation list. +// The item path read none of it, so describe → exec made every item visible. +func TestNavigationListItem_VisibleRolesRoundTrip(t *testing.T) { + stored := map[string]any{ + "$Type": "Forms$NavigationList", + "Name": "nl", + "Items": []any{int32(2), map[string]any{ + "$Type": "Forms$NavigationListItem", + "Action": map[string]any{"$Type": "Forms$NoAction"}, + "ConditionalVisibilitySettings": map[string]any{ + "$Type": "Forms$ConditionalVisibilitySettings", + "Expression": "", + "ModuleRoles": []any{int32(1), "M.Admin"}, + }, + "Widgets": []any{int32(2)}, + }}, + } + var buf bytes.Buffer + ctx := (&Executor{}).newExecContext(context.Background()) + ctx.Output = &buf + outputWidgetMDLV3(ctx, parseRawWidget(ctx, stored)[0], 1) + got := buf.String() + if !strings.Contains(got, "item (Visible: roles (M.Admin))") { + t.Fatalf("describe output lacks the item's role visibility:\n%s", got) + } + prog, errs := visitor.Build("create snippet M.S {\n" + got + "}\n") + if len(errs) > 0 { + t.Fatalf("describe output does not parse: %v\n%s", errs, got) + } + nl := prog.Statements[0].(*ast.CreateSnippetStmtV3).Widgets[0] + item, err := visibleWhenPB("").buildNavigationListItemV3(nl.Children[0]) + if err != nil { + t.Fatalf("build: %v", err) + } + if item.ConditionalVisibility == nil || !reflect.DeepEqual(item.ConditionalVisibility.ModuleRoles, []string{"M.Admin"}) { + t.Errorf("item ConditionalVisibility = %+v, want ModuleRoles [M.Admin]", item.ConditionalVisibility) + } +} + +// The builder carries the roles into the settings element, alone or beside an +// expression or attribute condition — Studio Pro stores all three on one node. +func TestVisibleRoles_Builds(t *testing.T) { + for _, tc := range []struct { + name string + props map[string]any + check func(*pages.ConditionalVisibilitySettings) string + }{ + {"roles only", map[string]any{"VisibleRoles": []string{"M.Admin"}}, + func(c *pages.ConditionalVisibilitySettings) string { + if c.Expression != "" || c.Attribute != "" { + return "roles alone must store no condition" + } + return "" + }}, + {"with expression", map[string]any{"VisibleRoles": []string{"M.Admin"}, "VisibleIf": "false"}, + func(c *pages.ConditionalVisibilitySettings) string { + if c.Expression != "false" { + return "expression lost: " + c.Expression + } + return "" + }}, + {"with attribute", map[string]any{"VisibleRoles": []string{"M.Admin"}, + "VisibleWhen": &ast.VisibleWhenV3{Attribute: "IsLocal", Values: []string{"true"}}}, + func(c *pages.ConditionalVisibilitySettings) string { + if c.Attribute != "M.Job.IsLocal" { + return "attribute lost: " + c.Attribute + } + return "" + }}, + } { + t.Run(tc.name, func(t *testing.T) { + w := &ast.WidgetV3{Type: "container", Name: "c1", Properties: tc.props} + built, err := visibleWhenPB("M.Job").buildWidgetV3(w) + if err != nil { + t.Fatalf("build: %v", err) + } + cvs := built.(*pages.Container).ConditionalVisibility + if cvs == nil { + t.Fatal("no ConditionalVisibilitySettings — the role restriction was dropped") + } + if !reflect.DeepEqual(cvs.ModuleRoles, []string{"M.Admin"}) { + t.Errorf("ModuleRoles = %v, want [M.Admin]", cvs.ModuleRoles) + } + if msg := tc.check(cvs); msg != "" { + t.Error(msg) + } + }) + } +} diff --git a/mdl/executor/validate_grant_refs.go b/mdl/executor/validate_grant_refs.go index a3785e1a63..00646d12ff 100644 --- a/mdl/executor/validate_grant_refs.go +++ b/mdl/executor/validate_grant_refs.go @@ -205,6 +205,18 @@ func (g *grantResolver) check(stmt ast.Statement) []string { if s.GuestUserRole != "" { add(g.userRoleProblem(s.GuestUserRole)) } + case *ast.CreatePageStmtV3, *ast.CreateSnippetStmtV3, *ast.AlterPageStmt: + // `Visible: roles (…)` stores the names unresolved, as `create user + // role` does — a dangling one is CE1613 a build later (#1460). + forEachWidget(stmt, func(w *ast.WidgetV3, where string) { + for _, r := range widgetVisibleRoles(w) { + if qn, ok := splitQualified(r); ok { + if msg := g.moduleRoleProblem(qn); msg != "" { + add(where + ": Visible: roles: " + msg) + } + } + } + }) } return out } @@ -598,6 +610,12 @@ func grantStatementLabel(stmt ast.Statement) string { return fmt.Sprintf("create demo user '%s'", s.UserName) case *ast.AlterProjectSecurityStmt: return "alter app security" + case *ast.CreatePageStmtV3: + return "create page " + s.Name.String() + case *ast.CreateSnippetStmtV3: + return "create snippet " + s.Name.String() + case *ast.AlterPageStmt: + return "alter " + strings.ToLower(s.ContainerType) + " " + s.PageName.String() } return "statement" } diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 3b6a50fc32..18d30ce215 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -785,7 +785,7 @@ var staticWidgetKnownProps = func() map[string]bool { // keys the builders/visitor consume and the conditional-binding metadata "CaptionAttribute", "Collapsible", "DatabaseHost", "DefaultLanguage", "Footer", "FormOrientation", "HeaderMode", "LabelWidth", "Prefix", "ShowContentAs", "Title", - "Widget", "WidgetType", "ShowLabel", "VisibleIf", "VisibleWhen", "EditableIf", "DynamicClasses", + "Widget", "WidgetType", "ShowLabel", "VisibleIf", "VisibleWhen", "VisibleRoles", "EditableIf", "DynamicClasses", // vocabulary describe page emits (native widgets + datagrid columns) "Alignment", "AlternativeText", "ColumnClass", "ColumnWidth", "DesktopColumns", "DisplayAs", "Draggable", "DynamicCellClass", "HeightUnit", "Hidable", "ImageType", diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index cca96765bf..6aebecba23 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -2390,7 +2390,7 @@ func isBuiltinPropName(name string) bool { // The expression and attribute forms of the two above, as the visitor // lowers them. Stored on the CustomWidget itself, never in its Object; // MDL-WIDGET41 refuses them where the package declares no such setting. - "EditableIf", "VisibleIf", "VisibleWhen", + "EditableIf", "VisibleIf", "VisibleWhen", "VisibleRoles", "WidgetType", "DesignProperties", "Association", "CaptionAttribute", "Content", "RenderMode", "ContentParams", "CaptionParams", "ButtonStyle", "DesktopWidth", "DesktopColumns", "TabletColumns", diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index 63895e5377..c3071c4059 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -548,6 +548,12 @@ widgetPropertyV3 | VISIBLE COLON xpathConstraint /* @alias MDL-DEPR081 */ // Visible: [IsActive = true] | VISIBLE COLON qualifiedName IN LPAREN visibleValueV3 (COMMA visibleValueV3)* RPAREN // Visible: Status in (Running, empty) | Mod.Entity.Attr in (…) | VISIBLE COLON widgetAttributeRefV3 IN LPAREN visibleValueV3 (COMMA visibleValueV3)* RPAREN // Visible: $Param.Status in (Running) — read from a page/snippet parameter + // Studio Pro's "Visible for module roles" (ConditionalVisibilitySettings. + // ModuleRoles, by-name references). Independent of — and combinable with — + // the expression or attribute condition: a widget may carry both `Visible:` + // lines (mendixlabs/mxcli#1460). Ahead of the expression alternative, which + // would otherwise read `roles (…)` as a function call. + | VISIBLE COLON ROLES LPAREN visibleRoleListV3 RPAREN // Visible: roles (Mod.Admin, Mod.User) | VISIBLE COLON propertyValueV3 // Visible: false | VISIBLE COLON expression // Visible: $currentObject/Status = 'Open' | EDITABLE COLON xpathConstraint /* @alias MDL-DEPR081 */ // Editable: [Status != 'Closed'] @@ -846,6 +852,12 @@ selectionModeV3 ; // V3 Generic property value +// The module roles of `Visible: roles (…)`, a rule of its own so the +// widgetPropertyV3 context keeps its single qualifiedName accessor. +visibleRoleListV3 + : qualifiedName (COMMA qualifiedName)* + ; + propertyValueV3 : STRING_LITERAL | QUOTED_IDENTIFIER // "AttrName" — quoted attribute value for pluggable sub-props (chart series staticXAttribute) diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 68c7279586..9ddaf93ec9 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -891,6 +891,17 @@ func parseWidgetPropertyV3(ctx parser.IWidgetPropertyV3Context, widget *ast.Widg // Visible: [expression] (conditional visibility) or Visible: false (static) if propCtx.VISIBLE() != nil { + // `Visible: roles (Mod.A, …)` — Studio Pro's "Visible for module roles", + // kept under its own key so it combines with the condition forms below + // (mendixlabs/mxcli#1460). + if rl := propCtx.VisibleRoleListV3(); rl != nil { + var roles []string + for _, qn := range rl.AllQualifiedName() { + roles = append(roles, getQualifiedNameText(qn)) + } + widget.Properties["VisibleRoles"] = roles + return + } // `Visible: Attr in (v1, …)` — Studio Pro's "based on attribute value". if propCtx.IN() != nil { vw := &ast.VisibleWhenV3{} diff --git a/sdk/pages/pages_widgets.go b/sdk/pages/pages_widgets.go index a8d9804cdf..fcf80d3b32 100644 --- a/sdk/pages/pages_widgets.go +++ b/sdk/pages/pages_widgets.go @@ -93,8 +93,12 @@ type LayoutPlaceholder struct { // ConditionalVisibilitySettings represents visibility conditions. type ConditionalVisibilitySettings struct { model.BaseElement - Expression string `json:"expression,omitempty"` - ModuleRoles []model.ID `json:"moduleRoles,omitempty"` + Expression string `json:"expression,omitempty"` + // ModuleRoles is Studio Pro's "Visible for module roles": qualified + // Module.Role names (a BY_NAME reference list), stored as ModuleRoles [1, + // "Mod.Role", …] — TestApp WorkflowCommons. Combinable with Expression or + // Attribute; empty means the widget is not role-restricted. + ModuleRoles []string `json:"moduleRoles,omitempty"` // SourceVariable names the object Attribute is read from when that is not // the enclosing data context: a page or snippet parameter, for a widget // outside every data container (TestApp WorkflowCommons). nil is the null diff --git a/sdk/pages/pages_widgets_action.go b/sdk/pages/pages_widgets_action.go index e9081ed6d6..60371e4cc1 100644 --- a/sdk/pages/pages_widgets_action.go +++ b/sdk/pages/pages_widgets_action.go @@ -138,6 +138,10 @@ type NavigationListItem struct { Class string `json:"class,omitempty"` Style string `json:"style,omitempty"` DynamicClasses string `json:"dynamicClasses,omitempty"` + // ConditionalVisibility is the item's own Forms$ConditionalVisibilitySettings + // — an expression, attribute condition and/or module roles, as a widget's. + // nil is the null Studio Pro stores on an unrestricted item (#1460). + ConditionalVisibility *ConditionalVisibilitySettings `json:"conditionalVisibility,omitempty"` } // LinkButton represents a link-style button. From 8759ddac4da58938fbca4dad940b4ea7ff114c96 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:51:31 +0000 Subject: [PATCH 08/23] fix(check): resolve ALTER PAGE targets and inserted-widget references (mendixlabs/mxcli#1472) check -p --references resolved an ALTER PAGE/SNIPPET/LAYOUT down to its document only: the INSERT/REPLACE/DROP target widget and the flows, pages, snippets and entities the inserted or replacing widgets name went unchecked, so a typo passed check and stopped exec. validateAlterPageReferences now runs exec's own target resolver against the stored document (read-only) and validateWidgetReferences over the op widgets; targets the script adds or renames, and SET targets (the dry run's finding), stay silent. TestAlterPageRefs_*; stubbing the pass to return nil fails all five cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/validate.go | 4 + mdl/executor/validate_alter_page_refs.go | 177 ++++++++++++++++++ mdl/executor/validate_alter_page_refs_test.go | 131 +++++++++++++ 3 files changed, 312 insertions(+) create mode 100644 mdl/executor/validate_alter_page_refs.go create mode 100644 mdl/executor/validate_alter_page_refs_test.go diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index bb570e1beb..a3cdb2165c 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -482,6 +482,10 @@ func validateProgramWithWarnings(ctx *ExecContext, prog *ast.Program) ([]error, // no data container at all) a bare binding is written bare and the project // no longer loads — CREATE PAGE already refused this at check time. errors = append(errors, validateAlterUnscopedBindings(ctx, prog, sc)...) + // An ALTER PAGE's INSERT / REPLACE / DROP targets and the references its + // inserted widgets carry: only the document was resolved, so a typo in + // either passed check and stopped exec (mendixlabs/mxcli#1472). + errors = append(errors, validateAlterPageReferences(ctx, prog, sc)...) return errors, sc.warnings } diff --git a/mdl/executor/validate_alter_page_refs.go b/mdl/executor/validate_alter_page_refs.go new file mode 100644 index 0000000000..2f5e477e33 --- /dev/null +++ b/mdl/executor/validate_alter_page_refs.go @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/model" +) + +// ALTER PAGE / SNIPPET / LAYOUT was resolved down to its document and no +// further (mendixlabs/mxcli#1472). Two kinds of reference inside it passed +// `check -p --references` and were then refused by exec, or written for +// mxbuild to report as CE1613: +// +// - the widget an INSERT is placed against, a REPLACE replaces, a DROP +// removes — exec resolves each through the document's AlterTargetResolver +// before the operation runs, and stops on a miss; +// - the flows, pages, snippets and entities the inserted or replacing +// widgets name — CREATE PAGE resolved those (validateWidgetReferences), +// ALTER never did. +// +// Both are answered by what exec itself runs: the targets through +// resolveAlterPageTargets against the stored document (opened read-only, never +// saved), the widget references through validateWidgetReferences. A SET's +// target is not resolved here — validateAlterSetProperties dry-runs the SET and +// already reports it, and saying it twice reads as two problems. +// +// "Could not establish" stays silent: a document that will not open, a target +// the script itself puts on the document (or renames a widget to), and a grid +// column on a document the script adds widgets to (a column is addressed by a +// derived name, so a name match cannot say whether the script adds it). + +// validateAlterPageReferences reports the unresolvable targets and widget +// references of every ALTER PAGE / SNIPPET / LAYOUT in the script. +func validateAlterPageReferences(ctx *ExecContext, prog *ast.Program, sc *scriptContext) []error { + if prog == nil || !ctx.Connected() { + return nil + } + h, _ := getHierarchy(ctx) + added := widgetsTheScriptAddsTo(prog) + grows := documentsTheScriptAddsWidgetsTo(prog) + opened := map[model.ID]backend.PageMutator{} + + var errs []error + for _, stmt := range prog.Statements { + s, ok := stmt.(*ast.AlterPageStmt) + if !ok { + continue + } + kind := strings.ToLower(s.ContainerType) + if kind == "" { + kind = "page" + } + label := fmt.Sprintf("alter %s %s", kind, s.PageName.String()) + doc := s.PageName.String() + excluded := (kind == "page" || kind == "snippet") && carriedExclusion(ctx, kind, s.PageName, true) + + // The stored document, for the targets. None for a document the + // script creates, or one that does not resolve (validateAlterTarget's + // finding) or will not open. + var mutator backend.PageMutator + if h != nil && !alterTargetComesFromScript(sc, s) { + if unitID, _, _, err := resolveAlterPageUnit(ctx, s, h); err == nil { + m, seen := opened[unitID] + if !seen { + m, _ = ctx.Backend.OpenPageForMutation(unitID) + opened[unitID] = m + } + mutator = m + } + } + + // The references the inserted and replacing widgets carry. They need no + // stored document, so a document the script creates is covered too. + ws := alterOpWidgets(s) + if refErrors := validateWidgetReferences(ctx, ws, sc); len(refErrors) > 0 { + // An excluded document stays excluded through an ALTER, and is + // relaxed the way CREATE relaxes it. + if excluded { + if err := sc.relaxExcludedWidgetRefs(kind, doc, ws, refErrors); err != nil { + errs = append(errs, err) + } + } else { + errs = append(errs, mdlerrors.NewValidationf("%s has reference errors:\n - %s", + label, strings.Join(refErrors, "\n - "))) + } + } + + // The targets, against the stored document. + if mutator == nil { + continue + } + for _, op := range s.Operations { + if _, isSet := op.(*ast.SetPropertyOp); isSet { + continue // validateAlterSetProperties' finding + } + for _, ref := range alterPageOperationTargets(op) { + if ref.Widget != "" && added[doc][ref.Widget] { + continue + } + if ref.IsColumn() && grows[doc] { + continue + } + if _, err := mutator.ResolveAlterTarget(alterTargetOf(ref)); err != nil { + errs = append(errs, mdlerrors.NewValidationf("%s: %v", label, err)) + } + } + } + } + return errs +} + +// alterOpWidgets returns the widgets the INSERT and REPLACE operations of one +// statement carry. +func alterOpWidgets(s *ast.AlterPageStmt) []*ast.WidgetV3 { + var ws []*ast.WidgetV3 + for _, op := range s.Operations { + switch o := op.(type) { + case *ast.InsertWidgetOp: + ws = append(ws, o.Widgets...) + case *ast.ReplaceWidgetOp: + ws = append(ws, o.NewWidgets...) + } + } + return ws +} + +// widgetsTheScriptAddsTo names, per document, every widget an INSERT or +// REPLACE anywhere in the script puts on it (nested widgets included), and +// every name a SET renames a widget to. Not ordered, for the reason +// documentsTheScriptAddsWidgetsTo gives: a false refusal blocks a script that +// works, a missed one costs one unreported error exec still reports. +func widgetsTheScriptAddsTo(prog *ast.Program) map[string]map[string]bool { + out := map[string]map[string]bool{} + for _, stmt := range prog.Statements { + s, ok := stmt.(*ast.AlterPageStmt) + if !ok { + continue + } + doc := s.PageName.String() + names := out[doc] + if names == nil { + names = map[string]bool{} + out[doc] = names + } + var walk func(ws []*ast.WidgetV3) + walk = func(ws []*ast.WidgetV3) { + for _, w := range ws { + if w == nil { + continue + } + if w.Name != "" { + names[w.Name] = true + } + walk(w.Children) + } + } + walk(alterOpWidgets(s)) + for _, op := range s.Operations { + set, ok := op.(*ast.SetPropertyOp) + if !ok { + continue + } + for k, v := range set.Properties { + if str, isStr := v.(string); isStr && strings.EqualFold(k, "Name") { + names[str] = true + } + } + } + } + return out +} diff --git a/mdl/executor/validate_alter_page_refs_test.go b/mdl/executor/validate_alter_page_refs_test.go new file mode 100644 index 0000000000..2365a7ff03 --- /dev/null +++ b/mdl/executor/validate_alter_page_refs_test.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/backend/pagemutator" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1472 items 1 and 2: `check -p --references` resolved only an +// ALTER PAGE's document. The widget it inserts after, replaces or drops, and +// every flow / page / entity the inserted or replacing widgets name, went +// unresolved — "All references valid", then exec refused the script. + +func alterRefsCtx(t *testing.T) (*ExecContext, *countingDeps) { + t.Helper() + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "P_Scopes") + deps := &countingDeps{} + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{mkMicroflow(mod.ID, "DS_Ok")}, nil + }, + OpenPageForMutationFunc: func(unitID model.ID) (backend.PageMutator, error) { + return pagemutator.New(storedScopesPage(), unitID, deps), nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx, deps +} + +func checkAlterPageRefs(t *testing.T, ctx *ExecContext, src string) []error { + t.Helper() + prog := parseMDL(t, src) + sc := newScriptContext() + sc.collectDefinitions(prog) + return validateAlterPageReferences(ctx, prog, sc) +} + +func TestAlterPageRefs_Reported(t *testing.T) { + cases := map[string]struct { + src string + want []string + }{ + "missing insert target": { + `alter page MyModule.P_Scopes { insert after tbTypo { textbox t (Attribute: Name) } }`, + []string{"MyModule.P_Scopes", "tbTypo"}, + }, + "missing replace target": { + `alter page MyModule.P_Scopes { replace tbTypo with { textbox t (Attribute: Name) } }`, + []string{"tbTypo"}, + }, + "missing drop target": { + `alter page MyModule.P_Scopes { drop widget tbTypo }`, + []string{"tbTypo"}, + }, + "inserted widget's data source flow": { + `alter page MyModule.P_Scopes { insert after tbTop { dataview dvX (DataSource: microflow MyModule.NoSuchMf) { } } }`, + []string{"microflow not found: MyModule.NoSuchMf"}, + }, + "replacing widgets' action page and flow": { + `alter page MyModule.P_Scopes { replace tbTop with { + actionbutton b1 (Caption: 'x', Action: show page MyModule.NoSuchPage) + dataview dvY (DataSource: microflow MyModule.NoSuchMf2) { } + } }`, + []string{"page not found: MyModule.NoSuchPage", "microflow not found: MyModule.NoSuchMf2"}, + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + ctx, deps := alterRefsCtx(t) + errs := checkAlterPageRefs(t, ctx, c.src) + if len(errs) == 0 { + t.Fatalf("got no errors, want one naming %v", c.want) + } + var all []string + for _, e := range errs { + all = append(all, e.Error()) + } + joined := strings.Join(all, "\n") + for _, w := range c.want { + if !strings.Contains(joined, w) { + t.Errorf("errors %q do not name %q", joined, w) + } + } + if deps.saves != 0 { + t.Errorf("validation wrote to storage %d times, want 0", deps.saves) + } + }) + } +} + +// Controls: what exec resolves must stay clean. +func TestAlterPageRefs_Controls(t *testing.T) { + cases := map[string]string{ + "stored target, stored flow": `alter page MyModule.P_Scopes { + insert after tbTop { dataview dvX (DataSource: microflow MyModule.DS_Ok) { } } }`, + "target added earlier in the statement": `alter page MyModule.P_Scopes { + insert after tbTop { container cNew { textbox tInner (Attribute: Name) } } + insert after tInner { textbox t2 (Attribute: Name) } }`, + "target added by an earlier statement": `alter page MyModule.P_Scopes { insert after tbTop { textbox tNew (Attribute: Name) } }; + alter page MyModule.P_Scopes { replace tNew with { textbox t3 (Attribute: Name) } };`, + "target renamed by SET Name": `alter page MyModule.P_Scopes { set Name = 'tbRenamed' on tbTop }; + alter page MyModule.P_Scopes { insert after tbRenamed { textbox t4 (Attribute: Name) } };`, + "flow created by the script": `create microflow MyModule.DS_New () returns MyModule.Customer begin return empty; end; + alter page MyModule.P_Scopes { insert after tbTop { dataview dvX (DataSource: microflow MyModule.DS_New) { } } }`, + "page created by the script is resolved where it is written": `create page MyModule.P_New (Title: 'n', Layout: Atlas_Core.Atlas_Default) { }; + alter page MyModule.P_New { insert after anything { textbox t (Attribute: Name) } }`, + "SET target is the SET dry run's finding": `alter page MyModule.P_Scopes { set Caption = 'x' on tbTypo }`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + ctx, _ := alterRefsCtx(t) + if errs := checkAlterPageRefs(t, ctx, src); len(errs) != 0 { + t.Errorf("got %v, want none", errs) + } + }) + } +} From f82a05293db79c3a6d50e9c263445f29131bb643 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:53:03 +0000 Subject: [PATCH 09/23] fix(check): resolve DataGrid 2 column attributes against the grid's entity (mendixlabs/mxcli#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing checked that a datagrid column's Attribute exists on the grid's entity — the scope rule only asks whether a name belongs to ANOTHER entity in scope — so a typo passed check and exec and mxbuild reported CE1613 (measured on 11.13.0). Judged for a database-source grid in CREATE PAGE/SNIPPET and in ALTER PAGE (new grids, and columns inserted into a stored grid whose own source is not a flow); anything not established stays silent. A header filter's Attribute is not judged: the column builder never writes it. TestDataGridColumnAttrs_*; stubbing the walk fails all four reported cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/validate.go | 18 ++ mdl/executor/validate_alter_page_refs.go | 47 +++++- .../validate_datagrid_column_attrs.go | 128 +++++++++++++++ .../validate_datagrid_column_attrs_test.go | 155 ++++++++++++++++++ 4 files changed, 346 insertions(+), 2 deletions(-) create mode 100644 mdl/executor/validate_datagrid_column_attrs.go create mode 100644 mdl/executor/validate_datagrid_column_attrs_test.go diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index a3cdb2165c..e4861894dc 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -857,6 +857,16 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewValidationf("page '%s' has attribute binding errors:\n - %s", s.Name.String(), strings.Join(scopeErrors, "\n - ")) } + // A DataGrid 2 column or filter attribute the grid's entity lacks + // (mendixlabs/mxcli#1472) — CE1613, unless Mendix skips the document. + if colErrors := datagridColumnAttrErrors(pageWidgets, "", newColumnAttrJudge(ctx, sc)); len(colErrors) > 0 { + if s.Excluded || carriedExclusion(ctx, "page", s.Name, s.IsReplace || s.IsModify) { + sc.warnExcluded("page", s.Name.String(), colErrors) + } else { + return mdlerrors.NewValidationf("page '%s' has attribute binding errors:\n - %s", + s.Name.String(), strings.Join(colErrors, "\n - ")) + } + } case *ast.CreateLayoutStmt: // A layout's widgets name things too — a menu widget's menu document // above all, since layouts are where menu widgets live. Before @@ -911,6 +921,14 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewValidationf("snippet '%s' has attribute binding errors:\n - %s", s.Name.String(), strings.Join(scopeErrors, "\n - ")) } + if colErrors := datagridColumnAttrErrors(s.Widgets, "", newColumnAttrJudge(ctx, sc)); len(colErrors) > 0 { + if carriedExclusion(ctx, "snippet", s.Name, s.IsReplace || s.IsModify) { + sc.warnExcluded("snippet", s.Name.String(), colErrors) + } else { + return mdlerrors.NewValidationf("snippet '%s' has attribute binding errors:\n - %s", + s.Name.String(), strings.Join(colErrors, "\n - ")) + } + } case *ast.CreateWorkflowStmt: // Two reference passes. Missing targets first: a name that resolves to // nothing is the more basic error, and reporting "parameter not mapped" diff --git a/mdl/executor/validate_alter_page_refs.go b/mdl/executor/validate_alter_page_refs.go index 2f5e477e33..a2bc7ad9d4 100644 --- a/mdl/executor/validate_alter_page_refs.go +++ b/mdl/executor/validate_alter_page_refs.go @@ -45,6 +45,7 @@ func validateAlterPageReferences(ctx *ExecContext, prog *ast.Program, sc *script added := widgetsTheScriptAddsTo(prog) grows := documentsTheScriptAddsWidgetsTo(prog) opened := map[model.ID]backend.PageMutator{} + var judge columnAttrJudge var errs []error for _, stmt := range prog.Statements { @@ -60,8 +61,9 @@ func validateAlterPageReferences(ctx *ExecContext, prog *ast.Program, sc *script doc := s.PageName.String() excluded := (kind == "page" || kind == "snippet") && carriedExclusion(ctx, kind, s.PageName, true) - // The stored document, for the targets. None for a document the - // script creates, or one that does not resolve (validateAlterTarget's + // The stored document, for the targets and for the entity of a grid + // the statement inserts columns into. None for a document the script + // creates, or one that does not resolve (validateAlterTarget's // finding) or will not open. var mutator backend.PageMutator if h != nil && !alterTargetComesFromScript(sc, s) { @@ -91,6 +93,31 @@ func validateAlterPageReferences(ctx *ExecContext, prog *ast.Program, sc *script } } + // DataGrid 2 column attributes: of a grid the statement + // carries, and of the columns it inserts into a stored grid. + var colErrors []string + if len(ws) > 0 { + if judge == nil { + judge = newColumnAttrJudge(ctx, sc) + } + colErrors = datagridColumnAttrErrors(ws, "", judge) + for _, op := range s.Operations { + target, cols := alterColumnInsertion(op) + if mutator == nil || len(cols) == 0 || added[doc][target.Widget] { + continue + } + colErrors = append(colErrors, datagridColumnAttrErrors(cols, alterColumnsEntity(mutator, target.Widget), judge)...) + } + } + if len(colErrors) > 0 { + if excluded { + sc.warnExcluded(kind, doc, colErrors) + } else { + errs = append(errs, mdlerrors.NewValidationf("%s has attribute binding errors:\n - %s", + label, strings.Join(colErrors, "\n - "))) + } + } + // The targets, against the stored document. if mutator == nil { continue @@ -115,6 +142,22 @@ func validateAlterPageReferences(ctx *ExecContext, prog *ast.Program, sc *script return errs } +// alterColumnInsertion returns the grid target and the columns of an INSERT or +// REPLACE that exec routes down its DataGrid 2 column path, or no columns. +func alterColumnInsertion(op ast.AlterPageOperation) (ast.WidgetRef, []*ast.WidgetV3) { + switch o := op.(type) { + case *ast.InsertWidgetOp: + if o.Target.IsColumn() && allColumns(o.Widgets) { + return o.Target, o.Widgets + } + case *ast.ReplaceWidgetOp: + if o.Target.IsColumn() && allColumns(o.NewWidgets) { + return o.Target, o.NewWidgets + } + } + return ast.WidgetRef{}, nil +} + // alterOpWidgets returns the widgets the INSERT and REPLACE operations of one // statement carry. func alterOpWidgets(s *ast.AlterPageStmt) []*ast.WidgetV3 { diff --git a/mdl/executor/validate_datagrid_column_attrs.go b/mdl/executor/validate_datagrid_column_attrs.go new file mode 100644 index 0000000000..67267b1c78 --- /dev/null +++ b/mdl/executor/validate_datagrid_column_attrs.go @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" +) + +// A DataGrid 2 column's `Attribute:` was never resolved (mendixlabs/mxcli#1472). +// The scope rule (validatePluggableAttributeScopes) only asks whether a name +// belongs to ANOTHER entity in scope; a name that belongs to none passed check +// and exec, and mxbuild answered — measured on 11.13.0 — +// +// CE1613 "The selected attribute 'Administration.Account.NoSuchColumnAttr' +// no longer exists." at Columns (1/1) of data grid 2 'dg' +// +// A filter in the column's header is NOT judged, though the issue names it: it +// filters the column's own attribute, and the column builder does not write +// the filter's `Attribute:` at all — a valid one and a misspelt one alike are +// dropped (describe prints a bare `textfilter tf1`), and mxbuild reports +// nothing for either. Refusing the misspelt one would refuse a script that +// builds. +// +// The grid's entity is only taken from a `database` source, where it is the +// statement's own text. A flow, association or selection source leaves the +// columns unjudged: could-not-establish stays silent. So does a path or a +// qualified name, a system member (stored as a flag, not an attribute), an +// entity whose chain the index does not hold, and an entity the script alters, +// imports or produces without declaring its members. + +// columnAttrJudge answers "is attr provably not an attribute of entity". +type columnAttrJudge func(entity, attr string) bool + +// newColumnAttrJudge builds the judge over the stored domain model, overlaid +// with the entities this script declares. +func newColumnAttrJudge(ctx *ExecContext, sc *scriptContext) columnAttrJudge { + stored := checkAttributeIndex(ctx, nil) + overlay := checkAttributeIndex(ctx, sc) + return func(entity, attr string) bool { + if entity == "" || attr == "" || strings.ContainsAny(attr, "./$ ") || isSystemMemberName(attr) { + return false + } + if sc != nil { + if sc.alteredEntities[entity] || sc.indirectEntities[entity] || + sc.externalEntityModules[qualifiedNameModule(entity)] { + return false + } + if sc.entities[entity] && sc.entityAttrs[entity] == nil { + return false // a view or external entity: members not recorded + } + } + // A CREATE OR MODIFY keeps stored members it does not restate, so a + // member either side declares is enough. + if found, _ := stored.declares(entity, attr); found { + return false + } + found, known := overlay.declares(entity, attr) + return !found && known + } +} + +// isSystemMemberName reports the system members an entity stores as a flag +// rather than as an attribute, which the attribute index therefore lacks. +func isSystemMemberName(attr string) bool { + switch strings.ToLower(attr) { + case "createddate", "changeddate", "owner", "changedby", "id": + return true + } + return false +} + +// datagridColumnAttrErrors walks a widget tree and reports the DataGrid 2 +// column attributes that are not attributes of the grid's database-source +// entity. columnsEntity is the entity of the grid that +// top-level `column` widgets belong to (ALTER … INSERT INTO a stored grid), +// or "" when the widgets are not columns of a known grid. +func datagridColumnAttrErrors(widgets []*ast.WidgetV3, columnsEntity string, missing columnAttrJudge) []string { + var errs []string + report := func(where, attr, entity string) { + errs = append(errs, fmt.Sprintf( + "%s: `%s` is not an attribute of %s, the entity of the grid's data source — "+ + "mxbuild reports CE1613 \"The selected attribute '%s.%s' no longer exists.\"", + where, attr, entity, entity, attr)) + } + // gridEntity is the entity the widgets in ws are columns of, or "". + var walk func(ws []*ast.WidgetV3, gridName, gridEntity string) + walk = func(ws []*ast.WidgetV3, gridName, gridEntity string) { + for _, w := range ws { + if w == nil { + continue + } + kind := strings.ToLower(w.Type) + switch { + case kind == "datagrid": + entity := "" + if ds := w.GetDataSource(); ds != nil && ds.Type == "database" { + entity = ds.Reference + } + walk(w.Children, w.Name, entity) + continue + case kind == "column" && gridEntity != "": + attr := w.GetAttribute() + if missing(gridEntity, attr) { + report(fmt.Sprintf("datagrid `%s` column `%s`", gridName, columnLabel(w)), attr, gridEntity) + } + } + // Below a column, or any other widget, the grid's reach ends: the + // content is judged by the rules for the widgets it holds. + walk(w.Children, "", "") + } + } + walk(widgets, "", columnsEntity) + return errs +} + +// alterColumnsEntity is the entity a stored grid gives the columns an ALTER +// inserts into it, when that entity is the grid's own non-flow source — the +// entity exec builds the columns against (alterEntityContext). "" otherwise. +func alterColumnsEntity(mutator backend.PageMutator, grid string) string { + if mf, nf := mutator.EnclosingDataSourceFlow(grid, true); mf != "" || nf != "" { + return "" + } + return mutator.EnclosingEntityForChildren(grid) +} diff --git a/mdl/executor/validate_datagrid_column_attrs_test.go b/mdl/executor/validate_datagrid_column_attrs_test.go new file mode 100644 index 0000000000..0f840ed326 --- /dev/null +++ b/mdl/executor/validate_datagrid_column_attrs_test.go @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1472 item 3: a DataGrid 2 column's Attribute, and a header +// filter's, were never resolved. Measured on 11.13.0, the column is CE1613 +// "The selected attribute 'Administration.Account.NoSuchColumnAttr' no longer +// exists." at Columns (1/1) of data grid 2 'dg'. + +func columnAttrsCtx(t *testing.T, gridFlow string) *ExecContext { + t.Helper() + mod := mkModule("MyModule") + cust := mkEntity(mod.ID, "Customer") + cust.Attributes = []*domainmodel.Attribute{{Name: "Name"}, {Name: "Email"}} + pg := mkPage(mod.ID, "P_Grid") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { + return []*domainmodel.DomainModel{mkDomainModel(mod.ID, cust)}, nil + }, + OpenPageForMutationFunc: func(unitID model.ID) (backend.PageMutator, error) { + return &mock.MockPageMutator{ + EnclosingEntityForChildrenFunc: func(string) string { return "MyModule.Customer" }, + EnclosingDataSourceFlowFunc: func(string, bool) (string, string) { + return gridFlow, "" + }, + }, nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func checkColumnAttrs(t *testing.T, ctx *ExecContext, src string) []string { + t.Helper() + prog := parseMDL(t, src) + sc := newScriptContext() + sc.collectDefinitions(prog) + var out []string + for _, stmt := range prog.Statements { + if err := validateWithContext(ctx, stmt, sc); err != nil { + out = append(out, err.Error()) + } + } + for _, err := range validateAlterPageReferences(ctx, prog, sc) { + out = append(out, err.Error()) + } + return out +} + +func TestDataGridColumnAttrs_Reported(t *testing.T) { + cases := map[string]struct { + src string + want []string + }{ + "create page column": { + `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { + column (Attribute: NoSuchColumnAttr, Caption: 'x') { textfilter tf1 } + } + }`, + []string{"NoSuchColumnAttr", "MyModule.Customer", "CE1613"}, + }, + "create snippet column": { + `create snippet MyModule.S_New { + datagrid dg (DataSource: database from MyModule.Customer) { column (Attribute: Nmae) } + }`, + []string{"`Nmae` is not an attribute of MyModule.Customer"}, + }, + "alter page inserts a new grid": { + `alter page MyModule.P_Grid { insert after anything { + datagrid dg2 (DataSource: database from MyModule.Customer) { column (Attribute: Nmae) } } }`, + []string{"`Nmae`"}, + }, + "alter page inserts a column into a stored grid": { + `alter page MyModule.P_Grid { insert after dgStored.Name { column (Attribute: Nmae) } }`, + []string{"`Nmae`"}, + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + got := strings.Join(checkColumnAttrs(t, columnAttrsCtx(t, ""), c.src), "\n") + for _, w := range c.want { + if !strings.Contains(got, w) { + t.Errorf("errors %q do not name %q", got, w) + } + } + }) + } +} + +func TestDataGridColumnAttrs_Controls(t *testing.T) { + cases := map[string]string{ + "existing attributes": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { + column (Attribute: Name) { textfilter tf1 (Attribute: Email) } + } }`, + // Measured on 11.13.0: the column builder writes no filter attribute + // (the filter filters the column's), so a misspelt one builds clean. + "a header filter's attribute is not written": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { + column (Attribute: Name) { textfilter tf1 (Attribute: NoSuchFilterAttr) } + } }`, + "case-insensitive like the builder": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { column (Attribute: name) } }`, + "association path": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { column (Attribute: Customer_Order/Total) } }`, + "system member": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { column (Attribute: createdDate) } }`, + "flow source is not established": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: microflow MyModule.DS) { column (Attribute: Whatever) } }`, + "entity unknown to the index": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from Other.Thing) { column (Attribute: Whatever) } }`, + "attribute added by the script": `alter entity MyModule.Customer add attribute Phone: String(20); + create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Customer) { column (Attribute: Phone) } }`, + "entity declared by the script": `create entity MyModule.Order (Total: Decimal); + create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + datagrid dg (DataSource: database from MyModule.Order) { column (Attribute: Total) } }`, + "layout grid column is not a grid column": `create page MyModule.P_New (Title: 'x', Layout: Atlas_Core.Atlas_Default) { + dataview dv (DataSource: database from MyModule.Customer) { + layoutgrid lg { row { column (DesktopWidth: 6) { textbox tb (Attribute: Name) } } } } }`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + for _, e := range checkColumnAttrs(t, columnAttrsCtx(t, ""), src) { + if strings.Contains(e, "is not an attribute of") { + t.Errorf("got %q, want no column attribute finding", e) + } + } + }) + } + // A stored grid sourced by a flow: its entity is not established here. + for _, e := range checkColumnAttrs(t, columnAttrsCtx(t, "MyModule.DS_Grid"), + `alter page MyModule.P_Grid { insert after dgStored.Name { column (Attribute: Nmae) } }`) { + if strings.Contains(e, "is not an attribute of") { + t.Errorf("flow-sourced stored grid: got %q, want silence", e) + } + } +} From 51738d9a348692cdd339a0c98b068186197c51f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 12:57:37 +0000 Subject: [PATCH 10/23] fix(check): resolve the members an access rule's WHERE names (mendixlabs/mxcli#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grantResolver resolved a grant's entity, roles and member list but never its XPath, so `where [M.NoSuchAssoc/…]` passed check and built to CE1613 at the access rule (measured on 11.13.0, security level Prototype). The constraint is now judged as exec stores it, by the walk the widget and retrieve checks use; its script-aware resolver is factored out of the retrieve check, which also stops judging an entity the script ALTERs (the control caught that as a false refusal). TestCheckGrantXPath_*; removing the call fails both reported cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/check_grant_xpath_pedapp_test.go | 68 +++++++++++++++++++ mdl/executor/validate_grant_refs.go | 45 ++++++++++++ mdl/executor/validate_retrieve_members.go | 42 ++++++++---- 3 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 mdl/executor/check_grant_xpath_pedapp_test.go diff --git a/mdl/executor/check_grant_xpath_pedapp_test.go b/mdl/executor/check_grant_xpath_pedapp_test.go new file mode 100644 index 0000000000..f89076ca38 --- /dev/null +++ b/mdl/executor/check_grant_xpath_pedapp_test.go @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// mendixlabs/mxcli#1472 item 4: the WHERE of an access rule was never +// resolved. Measured on 11.13.0 at security level Prototype, the reported +// script built to +// +// CE1613 "The selected association 'Administration.NoSuchAssoc' no longer +// exists." at Access rule of entity 'Administration.Account' +// +// while check printed "All references valid". Run on the real path against +// the Studio Pro-authored PedApp fixture, as the other grant checks are. +func TestCheckGrantXPath_UnknownMemberIsRefused(t *testing.T) { + cases := []struct{ name, src, want string }{ + {"the reported shape: an association path that names nothing", + "grant read * on entity Administration.Account to Administration.User where [Administration.NoSuchAssoc/Administration.Account/NoSuchAttr = 'x'];", + `association "Administration.NoSuchAssoc"`}, + {"a bare attribute the entity does not have", + "grant read * on entity Administration.Account to Administration.User where [NoSuchAttr = 'x'];", + `attribute "NoSuchAttr"`}, + } + exec, _, dir := openPedAppCopy(t) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := strings.Join(agreeCheck(t, exec, dir, "mdl 1;\n"+c.src), "\n") + if !strings.Contains(got, c.want) || !strings.Contains(got, "access rule") { + t.Errorf("check --references must report %q on the access rule, reported:\n%s", c.want, got) + } + }) + } +} + +func TestCheckGrantXPath_ResolvableMembersPass(t *testing.T) { + cases := []struct{ name, src string }{ + {"own attribute", + "grant read * on entity Administration.Account to Administration.User where [FullName = 'x'];"}, + {"attribute inherited from System.User", + "grant read * on entity Administration.Account to Administration.User where [Name = 'x'];"}, + {"implicit member", + "grant read * on entity Administration.Account to Administration.User where [id = '[%CurrentUser%]'];"}, + {"association inherited from System.User", + "grant read * on entity Administration.Account to Administration.User where [System.UserRoles = '[%UserRole_User%]'];"}, + {"attribute the script adds", + "alter entity Administration.Account add attribute Region: String(20);\n" + + "grant read * on entity Administration.Account to Administration.User where [Region = 'x'];"}, + {"association and entity the script creates", + "create persistent entity MyFirstModule.Thing (Name: String(10));\n" + + "create association Administration.Account_Thing from Administration.Account to MyFirstModule.Thing;\n" + + "grant read * on entity Administration.Account to Administration.User where [Administration.Account_Thing/MyFirstModule.Thing/Name = 'x'];"}, + {"entity the script creates", + "create persistent entity MyFirstModule.Thing (Name: String(10));\n" + + "grant read * on entity MyFirstModule.Thing to MyFirstModule.User where [Name = 'x'];"}, + } + exec, _, dir := openPedAppCopy(t) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := agreeCheck(t, exec, dir, "mdl 1;\n"+c.src); len(got) != 0 { + t.Fatalf("a resolvable constraint was refused:\n%s", strings.Join(got, "\n")) + } + }) + } +} diff --git a/mdl/executor/validate_grant_refs.go b/mdl/executor/validate_grant_refs.go index a3785e1a63..604d8991f8 100644 --- a/mdl/executor/validate_grant_refs.go +++ b/mdl/executor/validate_grant_refs.go @@ -79,6 +79,7 @@ type grantResolver struct { userRolesRead bool entities map[string]bool docs map[string]map[string]bool + xpath *grantXPathResolver // built on the first WHERE } func (g *grantResolver) recordLater(stmt ast.Statement) { @@ -144,6 +145,7 @@ func (g *grantResolver) check(stmt ast.Statement) []string { roles(s.Roles, true) if entityOK { add(g.memberProblem(s.Entity, s.Rights)) + add(g.xpathProblem(s.Entity, s.XPathConstraint)) } case *ast.RevokeEntityAccessStmt: g.entityTargetProblem(s.Entity, &out) @@ -467,6 +469,49 @@ func (g *grantResolver) userRoleProblem(name string) string { // with "entity … has no member(s) …". Only a stored entity is judged, and not // one the script alters: a member list built from a declaration or an altered // entity would be a second, drifting copy of exec's. +// xpathProblem resolves the members an access rule's WHERE names, against the +// granted entity (mendixlabs/mxcli#1472). The constraint is judged as exec +// stores it (accessRuleXPathConstraint), by the walk the widget and retrieve +// constraints use, over the project and what the whole script declares — exec +// writes the rule unresolved, so a member created further down is no defect. +// Measured on 11.13.0 at security level Prototype: +// +// where [Administration.NoSuchAssoc/…] CE1613 "The selected association +// 'Administration.NoSuchAssoc' no longer exists." at Access rule of entity +// +// (At level Off mxbuild does not check access rules at all; the rule is still +// dangling, and fails the build the day security is switched on.) +func (g *grantResolver) xpathProblem(qn ast.QualifiedName, xpath string) string { + if strings.TrimSpace(xpath) == "" { + return "" + } + name := qn.String() + if g.whole.indirectEntities[name] { + return "" // members not known to the script + } + if g.xpath == nil { + m, resolve := scriptXPathResolver(g.ctx, g.whole) + g.xpath = &grantXPathResolver{model: m, resolve: resolve} + } + stored := accessRuleXPathConstraint(g.ctx, xpath, name) + var bad []string + for _, b := range unresolvableXPathStepsWith(g.ctx, g.xpath.model, stored, name, g.xpath.resolve) { + bad = append(bad, fmt.Sprintf("%s %q", b.kind, b.name)) + } + if len(bad) == 0 { + return "" + } + return fmt.Sprintf("the access rule's constraint names %s, which %s neither an attribute nor an association of %s — "+ + "mxbuild reports CE1613 \"The selected … no longer exists\" at the access rule", + strings.Join(bad, ", "), plural(len(bad), "is", "are"), name) +} + +// grantXPathResolver caches the XPath walk's inputs for a script's grants. +type grantXPathResolver struct { + model *scriptXPathModel + resolve func(entityQN, member string) memberResolution +} + func (g *grantResolver) memberProblem(qn ast.QualifiedName, rights []ast.EntityAccessRight) string { name := qn.String() if g.whole.entities[name] || g.whole.indirectEntities[name] || g.whole.alteredEntities[name] { diff --git a/mdl/executor/validate_retrieve_members.go b/mdl/executor/validate_retrieve_members.go index a5d75a1b0a..dd0e05661c 100644 --- a/mdl/executor/validate_retrieve_members.go +++ b/mdl/executor/validate_retrieve_members.go @@ -33,11 +33,37 @@ func validateRetrieveMembers(ctx *ExecContext, retrieves []retrieveConstraintRef if len(retrieves) == 0 { return nil } + m, resolve := scriptXPathResolver(ctx, sc) + + var errs []string + for _, r := range retrieves { + if r.entity == "" || r.stored == "" { + continue + } + for _, bad := range unresolvableXPathStepsWith(ctx, m, r.stored, r.entity, resolve) { + errs = append(errs, fmt.Sprintf( + "retrieve from %s: the constraint names %q, which is neither an attribute nor an association of it — mxbuild rejects the constraint (CE0161 \"Error(s) in XPath constraint\")%s", + r.entity, bad.name, systemMemberSpellingHint(bad.name))) + } + } + return errs +} + +// scriptXPathResolver is the model and bare-member lookup an XPath member walk +// (unresolvableXPathStepsWith) needs to judge a constraint against the project +// AND the entities and associations the script declares. Shared by the +// microflow retrieve check and the access-rule check (mendixlabs/mxcli#1472). +func scriptXPathResolver(ctx *ExecContext, sc *scriptContext) (*scriptXPathModel, func(entityQN, member string) memberResolution) { m := &scriptXPathModel{base: &execXPathModel{ctx: ctx}, ctx: ctx, sc: sc} var assocs map[string]string // unqualified association name -> qualified, loaded on demand resolve := func(entityQN, member string) memberResolution { + // An entity the script alters may gain the member there: what the + // ALTER leaves is not worked out, so its members go unjudged. + if sc.alteredEntities[entityQN] { + return memberUnknown + } decl := sc.entityDecls[entityQN] - if decl == nil || sc.alteredEntities[entityQN] { + if decl == nil { return resolveMemberOnEntity(ctx, entityQN, member) } // Declared by the script: its attribute list is what exec writes. An @@ -63,19 +89,7 @@ func validateRetrieveMembers(ctx *ExecContext, retrieves []retrieveConstraintRef } return memberMissing } - - var errs []string - for _, r := range retrieves { - if r.entity == "" || r.stored == "" { - continue - } - for _, bad := range unresolvableXPathStepsWith(ctx, m, r.stored, r.entity, resolve) { - errs = append(errs, fmt.Sprintf( - "retrieve from %s: the constraint names %q, which is neither an attribute nor an association of it — mxbuild rejects the constraint (CE0161 \"Error(s) in XPath constraint\")%s", - r.entity, bad.name, systemMemberSpellingHint(bad.name))) - } - } - return errs + return m, resolve } // scriptXPathModel answers xpathrefs.Model from the project AND the script. From 4a3d151660236400c8af21657e25d3e577ab0de4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:26:13 +0000 Subject: [PATCH 11/23] fix(check): judge an access rule's XPath members case-insensitively (mendixlabs/mxcli#1472) The corpus sweep of the access-rule WHERE check against PedApp refused doctype-tests/08, whose `status` attribute is constrained as [Status = 'Open']. Measured on 11.13.0 at security level Prototype, that builds clean, so a member found in another case is now found. Two controls added to TestCheckGrantXPath_ResolvableMembersPass; both fail without the fold. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/check_grant_xpath_pedapp_test.go | 6 ++++++ mdl/executor/validate_grant_refs.go | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/mdl/executor/check_grant_xpath_pedapp_test.go b/mdl/executor/check_grant_xpath_pedapp_test.go index f89076ca38..6549f916e1 100644 --- a/mdl/executor/check_grant_xpath_pedapp_test.go +++ b/mdl/executor/check_grant_xpath_pedapp_test.go @@ -53,6 +53,12 @@ func TestCheckGrantXPath_ResolvableMembersPass(t *testing.T) { "create persistent entity MyFirstModule.Thing (Name: String(10));\n" + "create association Administration.Account_Thing from Administration.Account to MyFirstModule.Thing;\n" + "grant read * on entity Administration.Account to Administration.User where [Administration.Account_Thing/MyFirstModule.Thing/Name = 'x'];"}, + // Measured on 11.13.0: builds clean (doctype-tests/08 writes this). + {"member spelled in another case", + "create persistent entity MyFirstModule.Thing (status: String(10));\n" + + "grant read * on entity MyFirstModule.Thing to MyFirstModule.User where [Status = 'Open'];"}, + {"stored member spelled in another case", + "grant read * on entity Administration.Account to Administration.User where [fullname = 'x'];"}, {"entity the script creates", "create persistent entity MyFirstModule.Thing (Name: String(10));\n" + "grant read * on entity MyFirstModule.Thing to MyFirstModule.User where [Name = 'x'];"}, diff --git a/mdl/executor/validate_grant_refs.go b/mdl/executor/validate_grant_refs.go index 604d8991f8..647de5e90c 100644 --- a/mdl/executor/validate_grant_refs.go +++ b/mdl/executor/validate_grant_refs.go @@ -491,7 +491,20 @@ func (g *grantResolver) xpathProblem(qn ast.QualifiedName, xpath string) string } if g.xpath == nil { m, resolve := scriptXPathResolver(g.ctx, g.whole) - g.xpath = &grantXPathResolver{model: m, resolve: resolve} + // Case-insensitive, as mxbuild is here: measured on 11.13.0, an + // attribute `status` constrained as [Status = 'Open'] builds clean + // (doctype-tests/08 does exactly that). + ix := checkAttributeIndex(g.ctx, g.whole) + folded := func(entityQN, member string) memberResolution { + r := resolve(entityQN, member) + if r == memberMissing { + if found, _ := ix.declares(entityQN, member); found { + return memberFound + } + } + return r + } + g.xpath = &grantXPathResolver{model: m, resolve: folded} } stored := accessRuleXPathConstraint(g.ctx, xpath, name) var bad []string From 7d1f5b0d8edb659d14f2c0f49f5f563dfec2b6c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:03:02 +0000 Subject: [PATCH 12/23] fix(check): resolve an entity event handler's microflow (mendixlabs/mxcli#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither CREATE ENTITY nor ALTER ENTITY … ADD EVENT HANDLER resolved the microflow the handler calls; exec does (buildEventHandlers) and refuses a missing one after the statements before it are written. Check now resolves it against the project and the flows the script creates; an unqualified name or an empty listing stays silent. TestEntityEventHandlerRefs_*; short-circuiting the check fails both reported cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/validate.go | 9 ++ .../validate_entity_event_handlers.go | 43 +++++++++ .../validate_entity_event_handlers_test.go | 95 +++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 mdl/executor/validate_entity_event_handlers.go create mode 100644 mdl/executor/validate_entity_event_handlers_test.go diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index e4861894dc..8b46756f2c 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -650,6 +650,10 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext } } } + // The microflows its event handlers call (mendixlabs/mxcli#1472). + if err := validateEntityEventHandlers(ctx, sc, s.Name.String(), s.EventHandlers); err != nil { + return err + } // Validate index columns for _, idx := range s.Indexes { for _, col := range idx.Columns { @@ -990,6 +994,11 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFound("module", s.Name.Module) } } + if s.Operation == ast.AlterEntityAddEventHandler && s.EventHandler != nil { + if err := validateEntityEventHandlers(ctx, sc, s.Name.String(), []ast.EventHandlerDef{*s.EventHandler}); err != nil { + return err + } + } // ADD/DROP ATTRIBUTE on a view entity is refused by exec; say so here // too, or check passes a script that exec stops halfway (#1173). if s.Operation == ast.AlterEntityAddAttribute || s.Operation == ast.AlterEntityDropAttribute { diff --git a/mdl/executor/validate_entity_event_handlers.go b/mdl/executor/validate_entity_event_handlers.go new file mode 100644 index 0000000000..33deb066ae --- /dev/null +++ b/mdl/executor/validate_entity_event_handlers.go @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" +) + +// validateEntityEventHandlers resolves the microflow each entity event handler +// calls (mendixlabs/mxcli#1472). exec resolves it (buildEventHandlers) and +// refuses a missing one with "microflow not found" — after the statements +// before it are written — while check never looked, so `alter entity … add +// event handler on before commit call M.Typo(…)` reported "All references +// valid". A microflow the script creates counts as found; an unqualified name, +// or a backend that lists no microflows, is not established and stays silent. +func validateEntityEventHandlers(ctx *ExecContext, sc *scriptContext, entity string, defs []ast.EventHandlerDef) error { + if len(defs) == 0 || !ctx.Connected() { + return nil + } + var known map[string]bool + for _, d := range defs { + qn := d.Microflow.String() + if d.Microflow.Module == "" || d.Microflow.Name == "" || sc.microflows[qn] { + continue + } + if known == nil { + known = buildMicroflowQualifiedNames(ctx) + } + if len(known) == 0 { + return nil // a backend that lists none cannot say one is missing + } + if !known[qn] { + return mdlerrors.NewNotFoundMsg("microflow", qn, fmt.Sprintf( + "entity %s: the %s %s event handler calls microflow %s, which was not found — exec refuses it", + entity, strings.ToLower(d.Moment), strings.ToLower(d.Event), qn)) + } + } + return nil +} diff --git a/mdl/executor/validate_entity_event_handlers_test.go b/mdl/executor/validate_entity_event_handlers_test.go new file mode 100644 index 0000000000..a35d23479d --- /dev/null +++ b/mdl/executor/validate_entity_event_handlers_test.go @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1472 item 5: an entity event handler's microflow was never +// resolved at check time. exec resolves it (buildEventHandlers) and refuses a +// missing one — after the statements before it are written. + +func eventHandlerCtx(t *testing.T, listed bool) *ExecContext { + t.Helper() + mod := mkModule("MyModule") + cust := mkEntity(mod.ID, "Customer") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { + return []*domainmodel.DomainModel{mkDomainModel(mod.ID, cust)}, nil + }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + if !listed { + return nil, nil + } + return []*microflows.Microflow{mkMicroflow(mod.ID, "BCo_Customer")}, nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func checkEventHandlers(t *testing.T, ctx *ExecContext, src string) []string { + t.Helper() + prog := parseMDL(t, src) + sc := newScriptContext() + sc.collectDefinitions(prog) + var out []string + for _, stmt := range prog.Statements { + if err := validateWithContext(ctx, stmt, sc); err != nil { + out = append(out, err.Error()) + } + } + return out +} + +func TestEntityEventHandlerRefs_Reported(t *testing.T) { + cases := map[string]string{ + "alter entity add event handler": `alter entity MyModule.Customer add event handler on before commit call MyModule.NoSuchHandler($currentObject) raise error;`, + "create entity with a handler": `create persistent entity MyModule.Order (Total: Decimal) + on after create call MyModule.NoSuchHandler($currentObject);`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + got := strings.Join(checkEventHandlers(t, eventHandlerCtx(t, true), src), "\n") + if !strings.Contains(got, "MyModule.NoSuchHandler") || !strings.Contains(got, "event handler") { + t.Errorf("got %q, want a finding naming the handler microflow", got) + } + }) + } +} + +func TestEntityEventHandlerRefs_Controls(t *testing.T) { + cases := map[string]string{ + "stored microflow": `alter entity MyModule.Customer add event handler on before commit call MyModule.BCo_Customer($currentObject) raise error;`, + "microflow the script creates": `create microflow MyModule.BCo_New ($Customer: MyModule.Customer) returns Boolean begin return true; end; + alter entity MyModule.Customer add event handler on before commit call MyModule.BCo_New($currentObject) raise error;`, + "drop event handler names no microflow to resolve": `alter entity MyModule.Customer drop event handler on before commit;`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + for _, e := range checkEventHandlers(t, eventHandlerCtx(t, true), src) { + if strings.Contains(e, "event handler") { + t.Errorf("got %q, want no event handler finding", e) + } + } + }) + } + // A backend that lists no microflows cannot say one is missing. + for _, e := range checkEventHandlers(t, eventHandlerCtx(t, false), + `alter entity MyModule.Customer add event handler on before commit call MyModule.Whatever($currentObject);`) { + if strings.Contains(e, "event handler") { + t.Errorf("empty listing: got %q, want silence", e) + } + } +} From 03288127c060eab98d0aaf59cad17420d5f1baf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:11:54 +0000 Subject: [PATCH 13/23] fix(check): resolve enumeration keys named in expressions (mendixlabs/mxcli#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Module.Enum.Key` in an expression was never resolved: the enumeration's values were read for E001 but the key itself was never looked up, so a missing one passed check and exec and mxbuild reported CE1613 "The selected enumeration value … no longer exists" (measured on 11.13.0). Every qualified name expression of every statement is now judged against the project's enumeration or the one the script creates; an enum the script alters, renames or moves, one not in the project, and an excluded flow stay quiet or warn. TestEnumValueRefs_*; disabling the walk fails all three reported cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/validate.go | 3 + mdl/executor/validate_enum_value_refs.go | 212 ++++++++++++++++++ mdl/executor/validate_enum_value_refs_test.go | 120 ++++++++++ 3 files changed, 335 insertions(+) create mode 100644 mdl/executor/validate_enum_value_refs.go create mode 100644 mdl/executor/validate_enum_value_refs_test.go diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 8b46756f2c..a22fd0ec6a 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -486,6 +486,9 @@ func validateProgramWithWarnings(ctx *ExecContext, prog *ast.Program) ([]error, // inserted widgets carry: only the document was resolved, so a typo in // either passed check and stopped exec (mendixlabs/mxcli#1472). errors = append(errors, validateAlterPageReferences(ctx, prog, sc)...) + // `Module.Enum.Key` in any expression: the key must be one of the + // enumeration's values (mendixlabs/mxcli#1472). + errors = append(errors, validateEnumValueRefs(ctx, prog, sc)...) return errors, sc.warnings } diff --git a/mdl/executor/validate_enum_value_refs.go b/mdl/executor/validate_enum_value_refs.go new file mode 100644 index 0000000000..6eed25d93b --- /dev/null +++ b/mdl/executor/validate_enum_value_refs.go @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "reflect" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" +) + +// `Module.Enum.Key` inside an expression was never resolved +// (mendixlabs/mxcli#1472). The enumeration was known — the expression checker +// reads its values to refuse a quoted string compared with one (E001) — but +// whether the KEY it names exists was never asked, so +// +// declare $p Enumeration(NanoflowCommons.Platform) = NanoflowCommons.Platform.NoSuchKey; +// +// reported "All references valid", and exec wrote the expression for mxbuild +// to refuse — measured on 11.13.0, CE1613 "The selected enumeration value +// 'NanoflowCommons.Platform.NoSuchKey' no longer exists." at the activity. +// +// Every expression a statement carries is walked, whatever slot holds it — a +// declare, an assignment, a condition, a call argument, a return — because the +// visitor gives every one of them the same shape: a qualified name of three +// parts is `Module` plus `Enum.Key`. Run in the reference tier, which exec runs +// too, so the two cannot disagree. +// +// Could-not-establish is silence: an enumeration neither the project nor the +// script defines (System's are not stored in the project), one the script +// ALTERs, renames or moves, and a name that is also an entity's (a qualified +// attribute in an XPath constraint has the same three parts). + +// validateEnumValueRefs reports the enumeration keys that name no value. +func validateEnumValueRefs(ctx *ExecContext, prog *ast.Program, sc *scriptContext) []error { + if prog == nil || !ctx.Connected() { + return nil + } + script, unjudged := scriptEnumerationValues(prog) + var stored map[string][]string // enum QN -> values, nil entry = not in the project + var entities map[string]bool + + values := func(enumQN string) ([]string, bool) { + if unjudged[enumQN] { + return nil, false + } + if v, ok := script[enumQN]; ok { + return v, true + } + if stored == nil { + stored = map[string][]string{} + } + v, seen := stored[enumQN] + if !seen { + mod, name, _ := strings.Cut(enumQN, ".") + if e := findEnumeration(ctx, mod, name); e != nil { + for _, ev := range e.Values { + v = append(v, ev.Name) + } + if v == nil { + v = []string{} + } + } + stored[enumQN] = v + } + if v == nil { + return nil, false + } + if entities == nil { + entities = buildEntityQualifiedNames(ctx) + } + if entities[enumQN] { + return nil, false + } + return v, true + } + + var errs []error + for i, stmt := range prog.Statements { + var bad []string + vals := map[string][]string{} + seen := map[string]bool{} + walkQualifiedNameExprs(stmt, func(qn ast.QualifiedName) { + enum, key, ok := strings.Cut(qn.Name, ".") + if !ok || qn.Module == "" || enum == "" || key == "" || strings.Contains(key, ".") { + return + } + enumQN := qn.Module + "." + enum + v, known := values(enumQN) + if !known || hasValueFold(v, key) || seen[qn.String()] { + return + } + seen[qn.String()] = true + bad = append(bad, qn.String()) + vals[enumQN] = v + }) + if len(bad) == 0 { + continue + } + var msgs []string + for _, b := range bad { + enumQN := b[:strings.LastIndex(b, ".")] + msgs = append(msgs, fmt.Sprintf("%s: %s has no value %s (its values: %s)", + b, enumQN, b[len(enumQN)+1:], strings.Join(vals[enumQN], ", "))) + } + if kind, name, excluded := excludedFlow(stmt); excluded { + sc.warnExcluded(kind, name, msgs) + continue + } + errs = append(errs, mdlerrors.NewValidationf("statement %d: unknown enumeration value "+ + "(mxbuild: CE1613 \"The selected enumeration value … no longer exists.\"):\n - %s", + i+1, strings.Join(msgs, "\n - "))) + } + return errs +} + +// scriptEnumerationValues returns the values of the enumerations the script +// creates (the last definition wins, as when it runs), and the enumerations +// whose values it changes in a way not worked out here. +func scriptEnumerationValues(prog *ast.Program) (map[string][]string, map[string]bool) { + values := map[string][]string{} + unjudged := map[string]bool{} + for _, stmt := range prog.Statements { + switch s := stmt.(type) { + case *ast.CreateEnumerationStmt: + v := make([]string, 0, len(s.Values)) + for _, ev := range s.Values { + v = append(v, ev.Name) + } + values[s.Name.String()] = v + case *ast.AlterEnumerationStmt: + unjudged[s.Name.String()] = true + case *ast.RenameStmt: + if strings.EqualFold(s.ObjectType, "enumeration") { + unjudged[s.Name.String()] = true + unjudged[s.Name.Module+"."+s.NewName] = true + } + case *ast.MoveStmt: + if s.DocumentType == ast.DocumentTypeEnumeration { + unjudged[s.Name.String()] = true + if s.TargetModule != "" { + unjudged[s.TargetModule+"."+s.Name.Name] = true + } + } + } + } + return values, unjudged +} + +// excludedFlow reports a flow document the statement creates as excluded, +// whose dangling references Mendix does not validate. +func excludedFlow(stmt ast.Statement) (kind, name string, excluded bool) { + switch s := stmt.(type) { + case *ast.CreateMicroflowStmt: + return "microflow", s.Name.String(), s.Excluded + case *ast.CreateNanoflowStmt: + return "nanoflow", s.Name.String(), s.Excluded + case *ast.CreateRuleStmt: + return "rule", s.Name.String(), s.Excluded + } + return "", "", false +} + +func hasValueFold(values []string, key string) bool { + for _, v := range values { + if strings.EqualFold(v, key) { + return true + } + } + return false +} + +// walkQualifiedNameExprs calls fn for every qualified-name expression in v, at +// any depth. +func walkQualifiedNameExprs(v any, fn func(ast.QualifiedName)) { + var walk func(rv reflect.Value) + walk = func(rv reflect.Value) { + switch rv.Kind() { + case reflect.Interface: + if !rv.IsNil() { + walk(rv.Elem()) + } + case reflect.Pointer: + if rv.IsNil() { + return + } + if q, ok := rv.Interface().(*ast.QualifiedNameExpr); ok { + fn(q.QualifiedName) + return + } + walk(rv.Elem()) + case reflect.Struct: + for i := 0; i < rv.NumField(); i++ { + if rv.Type().Field(i).IsExported() { + walk(rv.Field(i)) + } + } + case reflect.Slice, reflect.Array: + for i := 0; i < rv.Len(); i++ { + walk(rv.Index(i)) + } + case reflect.Map: + iter := rv.MapRange() + for iter.Next() { + walk(iter.Value()) + } + } + } + walk(reflect.ValueOf(v)) +} diff --git a/mdl/executor/validate_enum_value_refs_test.go b/mdl/executor/validate_enum_value_refs_test.go new file mode 100644 index 0000000000..1561e7feed --- /dev/null +++ b/mdl/executor/validate_enum_value_refs_test.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1472 item 6: `Module.Enum.Key` in an expression was never +// resolved, so a key the enumeration does not have passed check and exec. + +func enumValueCtx(t *testing.T) *ExecContext { + t.Helper() + mod := mkModule("MyModule") + status := mkEnumeration(mod.ID, "Status", "Open", "Closed") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListEnumerationsFunc: func() ([]*model.Enumeration, error) { return []*model.Enumeration{status}, nil }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func checkEnumValues(t *testing.T, ctx *ExecContext, src string) ([]error, []string) { + t.Helper() + prog := parseMDL(t, src) + sc := newScriptContext() + sc.collectDefinitions(prog) + errs := validateEnumValueRefs(ctx, prog, sc) + return errs, sc.warnings +} + +func TestEnumValueRefs_Reported(t *testing.T) { + cases := map[string]struct{ src, want string }{ + "declare default and comparison (the reported shape)": { + `create microflow MyModule.K6 () returns Boolean + begin + declare $p Enumeration(MyModule.Status) = MyModule.Status.NoSuchKey; + return $p = MyModule.Status.AlsoNoKey; + end;`, + "MyModule.Status.NoSuchKey", + }, + "nanoflow if condition": { + `create nanoflow MyModule.N ($s: Enumeration(MyModule.Status)) returns Boolean + begin + if $s = MyModule.Status.Opne then return true; else return false; end if; + end;`, + "MyModule.Status.Opne", + }, + "enumeration the script creates": { + `create enumeration MyModule.Color (Red 'Red', Blue 'Blue'); + create microflow MyModule.C () returns Boolean begin + declare $c Enumeration(MyModule.Color) = MyModule.Color.Green; + return true; + end;`, + "MyModule.Color.Green", + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + errs, _ := checkEnumValues(t, enumValueCtx(t), c.src) + var all []string + for _, e := range errs { + all = append(all, e.Error()) + } + got := strings.Join(all, "\n") + if !strings.Contains(got, c.want) { + t.Errorf("got %q, want a finding naming %q", got, c.want) + } + }) + } + // Both keys of the reported shape, not just the first. + errs, _ := checkEnumValues(t, enumValueCtx(t), `create microflow MyModule.K6 () returns Boolean + begin + declare $p Enumeration(MyModule.Status) = MyModule.Status.NoSuchKey; + return $p = MyModule.Status.AlsoNoKey; + end;`) + if len(errs) != 1 || !strings.Contains(errs[0].Error(), "AlsoNoKey") || !strings.Contains(errs[0].Error(), "Open, Closed") { + t.Errorf("got %v, want one error naming both keys and the values", errs) + } +} + +func TestEnumValueRefs_Controls(t *testing.T) { + cases := map[string]string{ + "existing keys": `create microflow MyModule.OK () returns Boolean begin + declare $p Enumeration(MyModule.Status) = MyModule.Status.Open; + return $p = MyModule.Status.Closed; end;`, + "enumeration not in the project": `create microflow MyModule.OK () returns Boolean begin + return $x = System.WorkflowState.InProgress; end;`, + "value the script adds": `alter enumeration MyModule.Status add value Pending caption 'Pending'; + create microflow MyModule.OK () returns Boolean begin + declare $p Enumeration(MyModule.Status) = MyModule.Status.Pending; return true; end;`, + "enumeration the script creates, existing key": `create enumeration MyModule.Color (Red 'Red', Blue 'Blue'); + create microflow MyModule.C () returns Boolean begin + declare $c Enumeration(MyModule.Color) = MyModule.Color.Blue; return true; end;`, + "two-part name is not a key": `create microflow MyModule.OK () returns Boolean begin + declare $l List of MyModule.Status = empty; return true; end;`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + if errs, _ := checkEnumValues(t, enumValueCtx(t), src); len(errs) != 0 { + t.Errorf("got %v, want none", errs) + } + }) + } + // An excluded microflow is not validated by Mendix: a warning, not an error. + errs, warns := checkEnumValues(t, enumValueCtx(t), `@excluded + create microflow MyModule.X () returns Boolean begin + declare $p Enumeration(MyModule.Status) = MyModule.Status.Nope; return true; end;`) + if len(errs) != 0 || len(warns) != 1 { + t.Errorf("excluded: got errors %v, warnings %v; want one warning", errs, warns) + } +} From 5fdbbe531ebdc262fce9ca6de149109ef076d556 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:17:14 +0000 Subject: [PATCH 14/23] fix(check): resolve the entities a view entity's OQL reads from (mendixlabs/mxcli#1472) CREATE VIEW ENTITY checked column names and types only; an unknown FROM or JOIN entity merely left its columns untyped, and untyped columns are skipped, so the view passed check and exec and mxbuild reported CE0174 "Cannot resolve object name" (measured on 11.13.0). The sources (FROM, JOIN, end of a path join) are read by extractAliasMap and resolved against the project, System and what the script creates; an empty listing or a script-created module stays silent. TestViewSourceEntities_*; short-circuiting fails all three. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/executor/validate.go | 6 ++ mdl/executor/validate_view_source_entities.go | 84 ++++++++++++++++ .../validate_view_source_entities_test.go | 99 +++++++++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 mdl/executor/validate_view_source_entities.go create mode 100644 mdl/executor/validate_view_source_entities_test.go diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index a22fd0ec6a..2803d2d044 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -971,6 +971,12 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFound("module", s.Name.Module) } } + // The entities the query reads from (mendixlabs/mxcli#1472): first, + // because every column of a missing source goes untyped below. + if srcErrors := viewSourceEntityErrors(ctx, s.Query.RawQuery, sc); len(srcErrors) > 0 { + return mdlerrors.NewValidationf("view entity '%s':\n - %s", + s.Name.String(), strings.Join(srcErrors, "\n - ")) + } // An `.ID` column names an association, and the name has to be // free in the module — reported here because the fix is a rename, and a // rename is cheapest before the name spreads (FINDINGS §1). diff --git a/mdl/executor/validate_view_source_entities.go b/mdl/executor/validate_view_source_entities.go new file mode 100644 index 0000000000..2fcd112c28 --- /dev/null +++ b/mdl/executor/validate_view_source_entities.go @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/modelsdk/meta" +) + +// viewSourceEntityErrors reports the entities a view entity's OQL reads from — +// FROM, JOIN and the end of an association-path join — that neither the +// project nor the script has (mendixlabs/mxcli#1472). Before, an unknown +// source only made its columns infer no type, and an untyped column is +// skipped, so `from Administration.NoSuchEntity as a` passed check, exec wrote +// it, and mxbuild (11.13.0) answered CE0174 "Error(s) in OQL query: Cannot +// resolve object name 'Administration.NoSuchEntity'". +// +// The sources are read by extractAliasMap, the same reading the type inference +// uses. Could-not-establish is silence: a module the script creates, a module a +// bulk external-entity import fills, a backend that lists no entities, and a +// System entity the metadata does not list. +func viewSourceEntityErrors(ctx *ExecContext, oql string, sc *scriptContext) []string { + sources := map[string]bool{} + for _, qn := range extractAliasMap(oql) { + sources[qn] = true + } + if len(sources) == 0 { + return nil + } + var known map[string]bool + lowerKnown := map[string]bool{} + var missing []string + for qn := range sources { + mod, name, ok := strings.Cut(qn, ".") + if !ok || mod == "" || name == "" { + continue + } + if sc.modules[mod] || sc.externalEntityModules[mod] || sc.entities[qn] || sc.indirectEntities[qn] { + continue + } + if strings.EqualFold(mod, systemModuleName) { + if len(meta.SystemEntities) == 0 { + continue + } + if !isSystemEntityName(name) { + missing = append(missing, qn) + } + continue + } + if known == nil { + known = buildEntityQualifiedNames(ctx) + for k := range known { + lowerKnown[strings.ToLower(k)] = true + } + } + if len(known) == 0 { + return nil // a backend that lists none cannot say one is missing + } + if !lowerKnown[strings.ToLower(qn)] { + missing = append(missing, qn) + } + } + sort.Strings(missing) + var out []string + for _, qn := range missing { + out = append(out, fmt.Sprintf("the query reads from %s, which does not exist — mxbuild reports "+ + "CE0174 \"Error(s) in OQL query: Cannot resolve object name '%s'\"", qn, qn)) + } + return out +} + +// isSystemEntityName reports whether System declares an entity of that name +// (runtime-only rows included: this only decides whether to stay silent). +func isSystemEntityName(name string) bool { + for _, e := range meta.SystemEntities { + if strings.EqualFold(e.Name, name) { + return true + } + } + return false +} diff --git a/mdl/executor/validate_view_source_entities_test.go b/mdl/executor/validate_view_source_entities_test.go new file mode 100644 index 0000000000..d7a0524621 --- /dev/null +++ b/mdl/executor/validate_view_source_entities_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// mendixlabs/mxcli#1472 item 7: the entities a view entity's OQL reads from +// were never resolved — an unknown FROM entity inferred no type and was +// skipped, so the view passed check. + +func viewSourceCtx(t *testing.T, listed bool) *ExecContext { + t.Helper() + mod := mkModule("MyModule") + cust := mkEntity(mod.ID, "Customer") + cust.Attributes = []*domainmodel.Attribute{{Name: "Name"}} + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { + if !listed { + return nil, nil + } + return []*domainmodel.DomainModel{mkDomainModel(mod.ID, cust)}, nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + return ctx +} + +func checkViewSources(t *testing.T, ctx *ExecContext, src string) string { + t.Helper() + prog := parseMDL(t, src) + sc := newScriptContext() + sc.collectDefinitions(prog) + var out []string + for _, stmt := range prog.Statements { + if err := validateWithContext(ctx, stmt, sc); err != nil { + out = append(out, err.Error()) + } + } + return strings.Join(out, "\n") +} + +func TestViewSourceEntities_Reported(t *testing.T) { + cases := map[string]struct{ src, want string }{ + "unknown FROM entity (the reported shape)": { + `create view entity MyModule.K7 (Name: String) as select a.Name as Name from MyModule.NoSuchEntity as a;`, + "MyModule.NoSuchEntity", + }, + "unknown JOIN entity": { + `create view entity MyModule.K7 (Name: String) as select c.Name as Name from MyModule.Customer as c join MyModule.NoSuchOther as o on o.Name = c.Name;`, + "MyModule.NoSuchOther", + }, + "unknown end of an association-path join": { + `create view entity MyModule.K7 (Name: String) as select c.Name as Name from MyModule.Customer as c join c/MyModule.Customer_X/MyModule.NoSuchEnd as x;`, + "MyModule.NoSuchEnd", + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + got := checkViewSources(t, viewSourceCtx(t, true), c.src) + if !strings.Contains(got, c.want) || !strings.Contains(got, "does not exist") { + t.Errorf("got %q, want a finding naming %q", got, c.want) + } + }) + } +} + +func TestViewSourceEntities_Controls(t *testing.T) { + cases := map[string]string{ + "stored entity": `create view entity MyModule.V (Name: String) as select c.Name as Name from MyModule.Customer as c;`, + "entity the script creates": `create persistent entity MyModule.Order (Ref: String(20)); + create view entity MyModule.V (Ref: String(20)) as select o.Ref as Ref from MyModule.Order as o;`, + "System entity": `create view entity MyModule.V (Name: String) as select u.Name as Name from System.User as u;`, + "module the script creates": `create module Fresh; create persistent entity Fresh.Thing (Name: String(10)); + create view entity MyModule.V (Name: String(10)) as select t.Name as Name from Fresh.Other as t;`, + } + for name, src := range cases { + t.Run(name, func(t *testing.T) { + if got := checkViewSources(t, viewSourceCtx(t, true), src); strings.Contains(got, "does not exist") { + t.Errorf("got %q, want no source-entity finding", got) + } + }) + } + // A backend that lists no entities cannot say one is missing. + if got := checkViewSources(t, viewSourceCtx(t, false), + `create view entity MyModule.V (Name: String) as select a.Name as Name from MyModule.Whatever as a;`); strings.Contains(got, "does not exist") { + t.Errorf("empty listing: got %q, want silence", got) + } +} From a1025e030b1ebcbd833252c8b7f10a03224c9967 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:35:38 +0000 Subject: [PATCH 15/23] docs(check): record what check -p now resolves, and what it still does not (mendixlabs/mxcli#1472) The check-syntax skill said `-p` "resolves every reference"; it now lists the reference kinds #1472 added and the ones a pass still does not prove (built-in input attributes, attributes inside an ALTER PAGE insert, enum keys in page widget expressions). Plus the finding record for the seven kinds. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- ...eck-references-seven-unresolved-kinds.json | 1 + .claude/skills/mendix/check-syntax/SKILL.md | 30 ++++++++++++------- 2 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-13-check-references-seven-unresolved-kinds.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-check-references-seven-unresolved-kinds.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-check-references-seven-unresolved-kinds.json new file mode 100644 index 0000000000..7f520fe64c --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-13-check-references-seven-unresolved-kinds.json @@ -0,0 +1 @@ +{"date": "2026-10-13", "area": "mdl/executor", "symptom": "`mxcli check -p --references` printed \"All references valid\" for seven kinds of wrong reference that exec then refused or mxbuild reported (mendixlabs/mxcli#1472): an ALTER PAGE INSERT/REPLACE/DROP target widget and the flows/pages its inserted or replacing widgets name (exec: widget/microflow not found); a DataGrid 2 column Attribute (CE1613); an access rule's WHERE path (CE1613 at the access rule); an entity event handler's microflow (exec: microflow not found); `Module.Enum.Key` in an expression (CE1613 \"The selected enumeration value … no longer exists\"); and a view entity's FROM/JOIN entity (CE0174 \"Cannot resolve object name\").", "cause": "Each statement's reference check stopped at the document or entity level and never looked inside: ALTER PAGE resolved only its document, validateWidgetReferences ran only for CREATE page/layout/snippet, the column scope rule asked only whether a name belonged to ANOTHER entity, grantResolver never parsed the WHERE, neither entity statement looked at its handlers, exprcheck read enum values but never asked whether the key exists, and an unknown OQL source just made its columns untyped (and untyped columns are skipped).", "file": "mdl/executor/validate_alter_page_refs.go; mdl/executor/validate_datagrid_column_attrs.go; mdl/executor/validate_grant_refs.go (xpathProblem) + validate_retrieve_members.go (scriptXPathResolver); mdl/executor/validate_entity_event_handlers.go; mdl/executor/validate_enum_value_refs.go; mdl/executor/validate_view_source_entities.go; wired from mdl/executor/validate.go", "insight": "Probe every sibling: the reported kinds were seven faces of one shape — a check that resolves the container and not its contents — and each was closed by reusing what exec or a neighbouring rule already runs (exec's AlterTargetResolver against the stored page, validateWidgetReferences, the retrieve check's XPath walk, extractAliasMap) rather than restating it. Two measurements changed the plan: a DataGrid 2 header filter's Attribute is never written by the column builder (describe prints a bare `textfilter`, mxbuild is silent), so refusing a misspelt one would refuse a script that builds; and pedapp sits at security level Off, where mxbuild skips access rules entirely — the access-rule CE1613 only appears after `alter project security level prototype`. The pedapp control for the grant WHERE also caught a latent false refusal in the shared XPath resolver: an entity the script ALTERs was judged from its stored attribute list.", "refs": ["mdl/executor/validate_alter_page_refs_test.go", "mdl/executor/validate_datagrid_column_attrs_test.go", "mdl/executor/check_grant_xpath_pedapp_test.go", "mdl/executor/validate_entity_event_handlers_test.go", "mdl/executor/validate_enum_value_refs_test.go", "mdl/executor/validate_view_source_entities_test.go"], "ce": ["CE1613", "CE0174"], "rules": []} diff --git a/.claude/skills/mendix/check-syntax/SKILL.md b/.claude/skills/mendix/check-syntax/SKILL.md index 60a1590abc..959f633e82 100644 --- a/.claude/skills/mendix/check-syntax/SKILL.md +++ b/.claude/skills/mendix/check-syntax/SKILL.md @@ -11,15 +11,15 @@ description: "Validate MDL with `mxcli check` before presenting or executing it, | lines | section | |---|---| | 29-35 | When to Use This Skill | -| 36-286 | `exec` refuses what `check` rejects | -| 287-422 | Pre-Flight Validation Checklist | -| 423-473 | Two rules that only real validation used to catch | -| 474-520 | Validation Workflow | -| 521-542 | Script Execution Behavior | -| 543-603 | Script Organization | -| 604-656 | Troubleshooting Parse Errors | -| 657-714 | Studio Pro MCP — Verification Only | -| 715-720 | Related Skills | +| 36-296 | `exec` refuses what `check` rejects | +| 297-432 | Pre-Flight Validation Checklist | +| 433-483 | Two rules that only real validation used to catch | +| 484-530 | Validation Workflow | +| 531-552 | Script Execution Behavior | +| 553-613 | Script Organization | +| 614-666 | Troubleshooting Parse Errors | +| 667-724 | Studio Pro MCP — Verification Only | +| 725-730 | Related Skills | # MDL Syntax Validation Skill @@ -62,7 +62,17 @@ pages, microflows and icons — against that project. It reaches inside stored documents where a name can only be answered there: an `ALTER PAGE … SET` is dry-run against the page it edits, so a widget the page does not have, or a property the stored widget does not declare, is reported here rather than -stopping the script partway through `exec`. +stopping the script partway through `exec`. The same holds for an `INSERT` / +`REPLACE` / `DROP` target, and for the flows, pages and snippets the inserted +widgets name. Also resolved: a DataGrid 2 column's `Attribute:` on a `database` +grid, an access rule's `where […]` members, an entity event handler's +microflow, `Module.Enum.Key` in an expression, and a view entity's FROM/JOIN +entities (mendixlabs/mxcli#1472). + +**Not yet resolved** — a pass does not prove these: a built-in input widget's +`Attribute:` (`textbox (Attribute: Typo)`), any attribute inside an `ALTER +PAGE` insert, and an enumeration key inside a page widget's expression. +`mxcli docker check` is still the gate for those. ```bash mxcli check script.mdl # syntax + model-free rules From 06f61fd42c2fcffe1270012e1e8577c872297de5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 14:30:25 +0000 Subject: [PATCH 16/23] test(roundtrip): strike the page #1460 lets round-trip (mendixlabs/mxcli#1460) WorkflowCommons.WorkflowAttachment_New broke getput only on its role-based visibility; it now round-trips and the harness asks for its entry to go. Snip_WorkflowActivityRecord_Header stays listed for its other drifts, minus the module-role visibility it no longer has. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- mdl/roundtrip/testapp_allowlist_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index d2d558ad8c..3a15447150 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -140,7 +140,6 @@ var testAppKnownFailures = map[string]knownFailure{ "page WorkflowCommons.UserTask_Target": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.UserTask_TargetGroups": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.User_WorkflowGroup_Select": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "page WorkflowCommons.WorkflowAttachment_New": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes. putget fixed by mendixlabs/mxcli#151: describe printed its file manager as a comment, so exec dropped it"}, "page WorkflowCommons.WorkflowAttachment_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.WorkflowCommentHelper_Edit": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.WorkflowComment_Edit_Admin": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, @@ -176,7 +175,7 @@ var testAppKnownFailures = map[string]knownFailure{ "snippet WorkflowCommons.Snip_UserTask_UnassignedMessage": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_ActivityIcon": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_Details": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes; executes since a widget bound to a snippet parameter is described as $Param.Attr — getput not yet triaged"}, - "snippet WorkflowCommons.Snip_WorkflowActivityRecord_Header": {laws: []law{lawGetPut, lawPutGet}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); association-path template parameters, a module-role visibility and an expression parameter drift (#721)"}, + "snippet WorkflowCommons.Snip_WorkflowActivityRecord_Header": {laws: []law{lawGetPut, lawPutGet}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); association-path template parameters and an expression parameter drift (#721)"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_TaskDueDate_Warning": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp (#743); putget passes since a template parameter written as an expression is stored as one (#823 option C)"}, "snippet WorkflowCommons.Snip_WorkflowActivity_Details_ActivityType": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowAuditTrailRecord_Overview": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, From 43bbb568b652c8ddee5bd7e12d65fa0a4cc270dd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 14:36:34 +0000 Subject: [PATCH 17/23] docs(skills): write the row-visibility example as a full statement (mendixlabs/mxcli#1461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conformance gate parses every MDL block in a skill; the bare `row (…) { … }` fragment was one finding over the file's allowlist. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .../skills/mendix/create-page/reference/widgets.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.claude/skills/mendix/create-page/reference/widgets.md b/.claude/skills/mendix/create-page/reference/widgets.md index e19ddc7cf8..46f1a47d0e 100644 --- a/.claude/skills/mendix/create-page/reference/widgets.md +++ b/.claude/skills/mendix/create-page/reference/widgets.md @@ -271,7 +271,16 @@ column (desktopwidth: 8, tabletwidth: 6, phonewidth: 12) { ... } the condition on the row, or wrap the column's content in a `container (Visible: …)`. ```sql -row (Visible: $currentObject/Outcome != empty) { column (desktopwidth: autofill) { ... } } +mdl 1; +create page MyMod.Order_Detail (Title: 'Order', Layout: Atlas_Core.Atlas_Default, Params: ( $Order: MyMod.Order )) { + layoutgrid lg { + row (Visible: $Order/Outcome != empty) { + column (desktopwidth: autofill) { + dynamictext outcome (content: 'Outcome', rendermode: H3) + } + } + } +}; ``` Example: From d68e3f9fd84824172d4cdf3d530d6d3bbe15d7a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 14:47:01 +0000 Subject: [PATCH 18/23] docs(skills): keep custom-widgets under the 700-line body cap (mendixlabs/mxcli#1463) TestLargeSkillsWereSplit bounds a SKILL.md body at 700 lines; the native-keyword note took custom-widgets to 707. Same content, three lines. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .claude/skills/mendix/custom-widgets/SKILL.md | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/.claude/skills/mendix/custom-widgets/SKILL.md b/.claude/skills/mendix/custom-widgets/SKILL.md index fb82242c1b..4e0947793c 100644 --- a/.claude/skills/mendix/custom-widgets/SKILL.md +++ b/.claude/skills/mendix/custom-widgets/SKILL.md @@ -10,16 +10,16 @@ description: "MDL syntax for pluggable widgets in CREATE PAGE / ALTER PAGE — a | lines | section | |---|---| -| 27-148 | Any installed widget is named by its own name | -| 149-250 | Built-in Pluggable Widgets | -| 251-339 | Charts (Mendix Charts.mpk) | -| 340-487 | Adding a Third-Party Widget | -| 488-513 | Authoring over MCP (live Studio Pro) | -| 514-616 | .def.json Reference | -| 617-677 | Engine Internals | -| 678-693 | Verify & Debug | -| 694-705 | Common Mistakes | -| 706-717 | Key Source Files | +| 27-151 | Any installed widget is named by its own name | +| 152-253 | Built-in Pluggable Widgets | +| 254-342 | Charts (Mendix Charts.mpk) | +| 343-490 | Adding a Third-Party Widget | +| 491-516 | Authoring over MCP (live Studio Pro) | +| 517-619 | .def.json Reference | +| 620-680 | Engine Internals | +| 681-696 | Verify & Debug | +| 697-708 | Common Mistakes | +| 709-720 | Key Source Files | # Custom & Pluggable Widgets in MDL @@ -146,15 +146,8 @@ If a widget you have installed is not found, extract its definition: mxcli widget init -p app.mpr ``` -The keyword is the last segment of the widget id, lowercased — except for a -**native mobile** widget (`com.mendix.widget.native.*`), which is prefixed -`native`: `nativeimage`, `nativeradiobuttons`, `nativebadge`. Native packages -reuse the web names, and a bare `image`/`radiobuttons` used to take over the -web keyword (mendixlabs/mxcli#1463). A project definition never takes a -built-in keyword (`image`, `radiobuttons`, `textbox`, …) for a different -widget id: it is reported with a `warning:` line and stays reachable only as -`pluggablewidget ''` — delete an old `image.def.json` / -`radiobuttons.def.json` generated for a native widget and re-run `widget init`. +The keyword is the widget id's last segment; a **native** widget (`com.mendix.widget.native.*`) is prefixed `native` (`nativeimage`, `nativebadge`). +A project definition never takes a built-in keyword (`image`, `radiobuttons`, …) for another widget id: it warns and stays `pluggablewidget ''`; delete a stale native `image.def.json` / `radiobuttons.def.json` and re-run `widget init` (mendixlabs/mxcli#1463). ## Built-in Pluggable Widgets From 6a68c8b6d77dc9e0953b1b822d4a6f8dd73c1d89 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 14:47:04 +0000 Subject: [PATCH 19/23] docs(skills): move check's resolved-reference list into a reference file (mendixlabs/mxcli#1472) TestLargeSkillsWereSplit bounds a SKILL.md body at 700 lines, and check-syntax was already at the bound; the list of what check -p resolves (and does not) now lives in check-syntax/reference/references-resolved.md, linked from the paragraph it extends. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .claude/skills/mendix/check-syntax/SKILL.md | 30 +++++++------------ .../reference/references-resolved.md | 21 +++++++++++++ 2 files changed, 31 insertions(+), 20 deletions(-) create mode 100644 .claude/skills/mendix/check-syntax/reference/references-resolved.md diff --git a/.claude/skills/mendix/check-syntax/SKILL.md b/.claude/skills/mendix/check-syntax/SKILL.md index 959f633e82..1654f97dd6 100644 --- a/.claude/skills/mendix/check-syntax/SKILL.md +++ b/.claude/skills/mendix/check-syntax/SKILL.md @@ -11,15 +11,15 @@ description: "Validate MDL with `mxcli check` before presenting or executing it, | lines | section | |---|---| | 29-35 | When to Use This Skill | -| 36-296 | `exec` refuses what `check` rejects | -| 297-432 | Pre-Flight Validation Checklist | -| 433-483 | Two rules that only real validation used to catch | -| 484-530 | Validation Workflow | -| 531-552 | Script Execution Behavior | -| 553-613 | Script Organization | -| 614-666 | Troubleshooting Parse Errors | -| 667-724 | Studio Pro MCP — Verification Only | -| 725-730 | Related Skills | +| 36-286 | `exec` refuses what `check` rejects | +| 287-422 | Pre-Flight Validation Checklist | +| 423-473 | Two rules that only real validation used to catch | +| 474-520 | Validation Workflow | +| 521-542 | Script Execution Behavior | +| 543-603 | Script Organization | +| 604-656 | Troubleshooting Parse Errors | +| 657-714 | Studio Pro MCP — Verification Only | +| 715-720 | Related Skills | # MDL Syntax Validation Skill @@ -62,17 +62,7 @@ pages, microflows and icons — against that project. It reaches inside stored documents where a name can only be answered there: an `ALTER PAGE … SET` is dry-run against the page it edits, so a widget the page does not have, or a property the stored widget does not declare, is reported here rather than -stopping the script partway through `exec`. The same holds for an `INSERT` / -`REPLACE` / `DROP` target, and for the flows, pages and snippets the inserted -widgets name. Also resolved: a DataGrid 2 column's `Attribute:` on a `database` -grid, an access rule's `where […]` members, an entity event handler's -microflow, `Module.Enum.Key` in an expression, and a view entity's FROM/JOIN -entities (mendixlabs/mxcli#1472). - -**Not yet resolved** — a pass does not prove these: a built-in input widget's -`Attribute:` (`textbox (Attribute: Typo)`), any attribute inside an `ALTER -PAGE` insert, and an enumeration key inside a page widget's expression. -`mxcli docker check` is still the gate for those. +stopping the script partway through `exec`. What else it resolves, and what a pass still does not prove: [reference/references-resolved.md](reference/references-resolved.md). ```bash mxcli check script.mdl # syntax + model-free rules diff --git a/.claude/skills/mendix/check-syntax/reference/references-resolved.md b/.claude/skills/mendix/check-syntax/reference/references-resolved.md new file mode 100644 index 0000000000..af8170dca7 --- /dev/null +++ b/.claude/skills/mendix/check-syntax/reference/references-resolved.md @@ -0,0 +1,21 @@ +# What `check -p --references` resolves + +Beyond the page, microflow, entity and icon names a statement names directly, +`check -p --references` resolves these against the project (mendixlabs/mxcli#1472): + +- an `ALTER PAGE` `INSERT` / `REPLACE` / `DROP` target, and the flows, pages and + snippets the inserted or replacing widgets name (a `SET` target is dry-run + against the stored page); +- a DataGrid 2 column's `Attribute:` on a `database` grid; +- an access rule's `where […]` members; +- an entity event handler's microflow; +- `Module.Enum.Key` in an expression; +- a view entity's FROM / JOIN entities. + +## Not yet resolved + +A passing check does not prove these; `mxcli docker check` is still the gate: + +- a built-in input widget's `Attribute:` (`textbox (Attribute: Typo)`); +- any attribute inside an `ALTER PAGE` insert; +- an enumeration key inside a page widget's expression. From 57905c06324c8e432b3a75dea878e5f3640590a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 14:50:42 +0000 Subject: [PATCH 20/23] docs(skills): regenerate create-page's line index (mendixlabs/mxcli#1460) The visibility section grew, so the generated "Find it fast" ranges were stale; `make check-skill-index` (push-test) refuses that. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .claude/skills/mendix/create-page/SKILL.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude/skills/mendix/create-page/SKILL.md b/.claude/skills/mendix/create-page/SKILL.md index 9141df0921..012887b120 100644 --- a/.claude/skills/mendix/create-page/SKILL.md +++ b/.claude/skills/mendix/create-page/SKILL.md @@ -15,12 +15,12 @@ description: "CREATE PAGE syntax reference — parameters, variables, layouts, a | 47-196 | Syntax | | 197-267 | Basic Examples | | 268-289 | Modifying Existing Pages | -| 290-377 | Conditional Visibility and Editability | -| 378-541 | Known Limitations | -| 542-550 | Tips | -| 551-577 | Related Commands | -| 578-602 | PLUGGABLEWIDGET Escape Hatch | -| 603-606 | See Also | +| 290-393 | Conditional Visibility and Editability | +| 394-557 | Known Limitations | +| 558-566 | Tips | +| 567-593 | Related Commands | +| 594-618 | PLUGGABLEWIDGET Escape Hatch | +| 619-622 | See Also | # CREATE PAGE - MDL Syntax Guide From 3a0a2112f5c5acb68dfbee0ba80c1c47d7600197 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 15:04:33 +0000 Subject: [PATCH 21/23] test: pin alter-entity on a system-date index end to end (mendixlabs/mxcli#1451) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1451 reports `alter entity … add attribute` rewriting a Studio Pro index over (createdDate, changedDate) with every segment as `Type = Normal` and still no attribute pointer, which crashes mx check in IndexedAttribute.get_Attribute. The converter defect is the one #1401 fixed (indexFromGen dropped Type, indexToGen hard-coded Normal); its tests pinned the converter pair and describe, not the reported sequence. This adds that sequence through the real write path: seed the Studio Pro shape below the writer, add an attribute via UpdateEntity, read the raw segments back. Reverting either half of #1401 fails it with exactly the reported `Type = "Normal"`. writeUnitDoc is factored out of the #1119 index GUID helper so both tests share the unit write + ContentsHash update. Also a bug-test MDL and a finding recording that the owner/changedBy/ association segment types (Model SDK, 11.11.0) are not checkable by mx 11.12.5 or 11.14.0 at all. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SsRmTMzxstdFGNtP3HLzzc --- ...14-alter-system-date-index-regression.json | 1 + .../1451-alter-keeps-system-date-index.mdl | 24 ++ .../modelsdk/issue1119_index_guid_test.go | 9 +- .../issue1451_alter_system_index_test.go | 246 ++++++++++++++++++ 4 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-backend/2026-10-14-alter-system-date-index-regression.json create mode 100644 mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl create mode 100644 mdl/backend/modelsdk/issue1451_alter_system_index_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-14-alter-system-date-index-regression.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-14-alter-system-date-index-regression.json new file mode 100644 index 0000000000..709658733d --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-14-alter-system-date-index-regression.json @@ -0,0 +1 @@ +{"date": "2026-10-14", "area": "mdl/backend", "symptom": "`alter entity … add attribute` on an entity with a Studio Pro index over (createdDate, changedDate) rewrote each segment as `Type = Normal` with still no attribute pointer; mx check then crashed with ArgumentOutOfRangeException at IndexedAttribute.get_Attribute instead of reporting a CE (mendixlabs/mxcli#1451, reported on v0.24.0 / 0.26.0, Mendix 11.12.5).", "cause": "Same defect as #1401, already fixed on main when #1451 was filed: indexFromGen dropped IndexedAttribute.Type and indexToGen hard-coded \"Normal\". #1401's tests pinned the converter pair and describe, not the reported ALTER sequence through the real write path.", "file": "mdl/backend/modelsdk/issue1451_alter_system_index_test.go (regression only; fix in domainmodel.go indexFromGen / domainmodel_write.go indexToGen from #1401)", "insight": "Check whether the report predates a fix before writing one: the issue quoted main's code, but a grep of the cited line (`seg.SetType(\"Normal\")`) on the current tree showed it already gone, and the real repro on a blank 11.12.5 app passed. What earned its place was the end-to-end test: seed the Studio Pro shape by patching the unit below the writer (Type CreatedDate/ChangedDate, zero AttributePointer), alter through UpdateEntity, read the raw segments; reverting either half of #1401 makes it fail with exactly the reported `Type = \"Normal\"`. Real run, both variants: fixed binary 0 errors; binary with indexToGen forced to Normal reproduces the get_Attribute crash. Do not chase the owner/changedBy half of the title: the Model SDK (mendixmodelsdk 4.116.0) does add IndexedAttributeType Owner/ChangedBy/Association and an `association` reference (AssociationPointer) in 11.11.0, absent from our gen, and a rewrite does zero a stored AssociationPointer — but mx 11.12.5 AND 11.14.0 crash in Index.Check -> get_Attribute on ANY segment of those types (zero pointer, a dangling one, or one naming a real association in the unit), so no project holding one passes mx check today; carrying the pointer protects a shape with no checkable instance. Revisit when a Studio Pro-authored owner/association index exists to measure. The domain-model unit stores entities as DomainModels$EntityImpl, so a raw patch matching `DomainModels$Entity` silently finds nothing.", "refs": ["mdl/backend/modelsdk/issue1451_alter_system_index_test.go", "mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl", ".claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-index-on-system-changed-date.json", "docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md"]} diff --git a/mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl b/mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl new file mode 100644 index 0000000000..588546f23e --- /dev/null +++ b/mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl @@ -0,0 +1,24 @@ +mdl 1; +-- mendixlabs/mxcli#1451 — `alter entity … add attribute` on an entity indexed +-- on its createdDate / changedDate system members. +-- +-- Studio Pro stores such a segment as IndexedAttribute.Type = CreatedDate / +-- ChangedDate with no attribute pointer. The writer used to hard-code Type +-- Normal, so any rewrite of the entity turned it into an attribute segment +-- pointing at nothing, and mx check crashed: +-- ArgumentOutOfRangeException … at IndexedAttribute.get_Attribute +-- +-- mxcli exec mdl-examples/bug-tests/1451-alter-keeps-system-date-index.mdl -p app.mpr +-- mxcli docker check -p app.mpr -- 0 errors, no crash +-- mxcli -p app.mpr -c "describe entity BugTest1451.Item" +-- -- still prints: index (CreatedDate, ChangedDate); +create module BugTest1451; + +create persistent entity BugTest1451.Item ( + "Name": String(50), + "CreatedDate": AutoCreatedDate, + "ChangedDate": AutoChangedDate +) +index ("CreatedDate", "ChangedDate"); + +alter entity BugTest1451.Item add attribute "X": String(50); diff --git a/mdl/backend/modelsdk/issue1119_index_guid_test.go b/mdl/backend/modelsdk/issue1119_index_guid_test.go index 11ac61a911..d99b990001 100644 --- a/mdl/backend/modelsdk/issue1119_index_guid_test.go +++ b/mdl/backend/modelsdk/issue1119_index_guid_test.go @@ -209,7 +209,15 @@ func patchIndexGUIDsWith(t *testing.T, proj string, dmID model.ID, entityName st if patched == 0 { t.Fatal("no DomainModels$EntityIndex GUID found to seed; the index was not written") } + writeUnitDoc(t, proj, dmID, path, doc) + return patched +} +// writeUnitDoc writes a patched domain-model unit back to its file and updates +// the .mpr's ContentsHash for it, so a test can seed stored state below the +// writer (a shape only Studio Pro produces) without the project disowning it. +func writeUnitDoc(t *testing.T, proj string, dmID model.ID, path string, doc bson.D) { + t.Helper() out, err := bson.Marshal(doc) if err != nil { t.Fatalf("marshal patched unit: %v", err) @@ -241,7 +249,6 @@ func patchIndexGUIDsWith(t *testing.T, proj string, dmID model.ID, entityName st if n != 1 { t.Fatalf("ContentsHash update matched %d rows, want 1", n) } - return patched } // storedUnitIDHex renders a unit id the way the .mpr's Unit.UnitID blob holds diff --git a/mdl/backend/modelsdk/issue1451_alter_system_index_test.go b/mdl/backend/modelsdk/issue1451_alter_system_index_test.go new file mode 100644 index 0000000000..d337dd69ee --- /dev/null +++ b/mdl/backend/modelsdk/issue1451_alter_system_index_test.go @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "bytes" + "os" + "testing" + + "github.com/mendixlabs/mxcli/model" + genDm "github.com/mendixlabs/mxcli/modelsdk/gen/domainmodels" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// mendixlabs/mxcli#1451, reported as: "`alter entity … add attribute` writes the +// entity back, and an existing index that includes a system member … comes back +// corrupted: each such segment is written with `Type = Normal` and still no +// attribute pointer", after which mxbuild throws ArgumentOutOfRangeException in +// IndexedAttribute.get_Attribute. +// +// This is the reporter's sequence through the real write path. The index is +// seeded the way Studio Pro stores it — Type CreatedDate / ChangedDate, an +// all-zero AttributePointer — by patching the unit below the writer, so the +// test does not depend on mxcli being able to author the shape it then reads. +// The ALTER is the backend half of `alter entity … add attribute`: re-read the +// entity, append the attribute, write it back. +func TestIssue1451_AddAttributeKeepsSystemMemberIndexSegments(t *testing.T) { + proj := copyFixture(t) + + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + dmID, _ := richestEntity(t, b) + const entName = "Issue1451Item" + ent := &domainmodel.Entity{ + Name: entName, + Persistable: true, + HasCreatedDate: true, + HasChangedDate: true, + Attributes: []*domainmodel.Attribute{ + {Name: "Name", Type: &domainmodel.StringAttributeType{Length: 50}}, + }, + } + if err := b.CreateEntity(dmID, ent); err != nil { + t.Fatalf("CreateEntity: %v", err) + } + // Two ordinary segments, so the unit holds two IndexedAttributes to re-type. + stored := mustFindEntity(t, b, dmID, entName) + nameID := stored.Attributes[0].ID + stored.Indexes = []*domainmodel.Index{{ + Attributes: []*domainmodel.IndexAttribute{ + {AttributeID: nameID, Ascending: true}, + {AttributeID: nameID, Ascending: true}, + }, + }} + if err := b.UpdateEntity(dmID, stored); err != nil { + t.Fatalf("UpdateEntity (add index): %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + want := []string{"CreatedDate", "ChangedDate"} + seedSystemIndexSegments(t, proj, dmID, entName, want) + before := systemIndexSegments(t, proj, dmID, entName) + for i, s := range before { + if s.typ != want[i] || !s.zeroPointer { + t.Fatalf("seed did not take: segment %d = %+v, want Type %s with a zero pointer", i, s, want[i]) + } + } + + // alter entity … add attribute "X": String(50); + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + target := mustFindEntity(t, b2, dmID, entName) + target.Attributes = append(target.Attributes, &domainmodel.Attribute{ + Name: "X", Type: &domainmodel.StringAttributeType{Length: 50}, + }) + if err := b2.UpdateEntity(dmID, target); err != nil { + t.Fatalf("UpdateEntity (add attribute): %v", err) + } + if err := b2.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + after := systemIndexSegments(t, proj, dmID, entName) + if len(after) != len(before) { + t.Fatalf("index has %d segments after the ALTER, want %d", len(after), len(before)) + } + for i, s := range after { + if s.typ != before[i].typ { + t.Errorf("segment %d Type = %q after the ALTER, want %q", i, s.typ, before[i].typ) + } + if !s.zeroPointer { + t.Errorf("segment %d AttributePointer is no longer all-zero after the ALTER", i) + } + if s.id != before[i].id { + t.Errorf("segment %d $ID = %s after the ALTER, want %s", i, s.id, before[i].id) + } + } +} + +type indexSegment struct { + id string + typ string + zeroPointer bool +} + +// systemIndexSegments reads the entity's index segments from the stored unit. +func systemIndexSegments(t *testing.T, proj string, dmID model.ID, entName string) []indexSegment { + t.Helper() + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + defer func() { _ = b.Disconnect() }() + gdm, err := b.loadDomainModelGen(dmID) + if err != nil { + t.Fatalf("loadDomainModelGen: %v", err) + } + var out []indexSegment + for _, el := range gdm.EntitiesItems() { + ge, ok := el.(*genDm.Entity) + if !ok || ge.Name() != entName { + continue + } + for _, iel := range ge.IndexesItems() { + idx, ok := iel.(*genDm.Index) + if !ok { + continue + } + for _, sel := range idx.AttributesItems() { + seg, ok := sel.(*genDm.IndexedAttribute) + if !ok { + continue + } + out = append(out, indexSegment{ + id: string(seg.ID()), + typ: seg.Type(), + zeroPointer: rawKeyHex(t, seg.Raw(), "AttributePointer") == "00000000000000000000000000000000", + }) + } + } + } + if len(out) == 0 { + t.Fatalf("no index segments stored on %s", entName) + } + return out +} + +// seedSystemIndexSegments re-types the entity's stored index segments, in +// order, to types[i] and zeroes their AttributePointer — what Studio Pro stores +// for a segment on createdDate / changedDate, which is not an attribute. +func seedSystemIndexSegments(t *testing.T, proj string, dmID model.ID, entName string, types []string) { + t.Helper() + path := unitFilePath(t, proj, entName) + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read unit: %v", err) + } + var doc bson.D + if err := bson.Unmarshal(raw, &doc); err != nil { + t.Fatalf("unmarshal unit: %v", err) + } + + typeOf := func(d bson.D) string { + for _, e := range d { + if e.Key == "$Type" { + s, _ := e.Value.(string) + return s + } + } + return "" + } + patched := 0 + var patchSegments func(any) + patchSegments = func(v any) { + switch x := v.(type) { + case bson.D: + if typeOf(x) == "DomainModels$IndexedAttribute" && patched < len(types) { + for i, e := range x { + switch e.Key { + case "Type": + x[i].Value = types[patched] + case "AttributePointer": + if bin, ok := e.Value.(bson.Binary); ok { + x[i].Value = bson.Binary{Subtype: bin.Subtype, Data: make([]byte, 16)} + } + } + } + patched++ + return + } + for _, e := range x { + patchSegments(e.Value) + } + case bson.A: + for _, e := range x { + patchSegments(e) + } + } + } + // Only inside the named entity: other entities in the unit may be indexed too. + var findEntity func(any) + findEntity = func(v any) { + switch x := v.(type) { + case bson.D: + if typeOf(x) == "DomainModels$EntityImpl" { // storage name of DomainModels$Entity + for _, e := range x { + if e.Key == "Name" && e.Value == entName { + patchSegments(x) + return + } + } + } + for _, e := range x { + findEntity(e.Value) + } + case bson.A: + for _, e := range x { + findEntity(e) + } + } + } + findEntity(doc) + if patched != len(types) { + t.Fatalf("seeded %d index segments, want %d", patched, len(types)) + } + if bytes.Equal(raw, mustMarshal(t, doc)) { + t.Fatal("seeding changed nothing") + } + writeUnitDoc(t, proj, dmID, path, doc) +} + +func mustMarshal(t *testing.T, doc bson.D) []byte { + t.Helper() + out, err := bson.Marshal(doc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return out +} From 2df4beaa451fbf52f4f850e6200d3f492ae98905 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:27:25 +0000 Subject: [PATCH 22/23] ci: Bump actions/setup-java from 5 to 6 Bumps [actions/setup-java](https://github.com/actions/setup-java) from 5 to 6. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](https://github.com/actions/setup-java/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-java dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/nightly.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 3af281ce5a..5bdbdda112 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -98,7 +98,7 @@ jobs: # The 9.x runtime and its Gradle build need JDK 17; pin it rather than # relying on the runner image's default JAVA_HOME. - - uses: actions/setup-java@v5 + - uses: actions/setup-java@v6 with: distribution: temurin java-version: '17' From 5c98d270a68f959a96bd6e2c4e85762847c6a0ba Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 18:30:16 +0000 Subject: [PATCH 23/23] docs(skills): regenerate alter-page's line index The legacy-grid note (mendixlabs/mxcli#1459) grew the file without regenerating its "Find it fast" ranges, so `make check-skill-index` fails on main and on every branch that merges it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VKmbj53BC2vuUWYmgWgfQ5 --- .claude/skills/mendix/alter-page/SKILL.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index a3f7224558..e08426e560 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -13,14 +13,14 @@ description: "Modify an existing page or snippet's widget tree in place with ALT | 28-31 | Overview | | 32-56 | When to Use | | 57-107 | Syntax | -| 108-460 | Operations | -| 461-539 | Examples | -| 540-630 | DataGrid 2 columns: how to address them, and what you can set | -| 631-642 | Common Mistakes | -| 643-664 | Limitations — prefer binding at page creation (ledger finding #45) | -| 665-672 | Validation Checklist | -| 673-682 | Related Commands | -| 683-687 | Related Skills | +| 108-472 | Operations | +| 473-551 | Examples | +| 552-642 | DataGrid 2 columns: how to address them, and what you can set | +| 643-654 | Common Mistakes | +| 655-676 | Limitations — prefer binding at page creation (ledger finding #45) | +| 677-684 | Validation Checklist | +| 685-694 | Related Commands | +| 695-699 | Related Skills | # ALTER PAGE / ALTER SNIPPET - Modify Existing Pages and Snippets