From 4e82a99453ee3e92ab016620408b40acf2775737 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 17:57:26 +0000 Subject: [PATCH 01/12] refactor: move import-mapping root-shape helpers to mdl/types The lint rules cannot import the executor, and a model-level MDL-MAP04 (mendixlabs/mxcli#1319) needs to decide a mapping's root shape exactly the way the builder and the check-time rule do. The executor keeps thin delegating wrappers so its call sites and tests are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BCeVsRwSYaVoEtimYbKJAi --- mdl/executor/cmd_microflows_builder_calls.go | 64 ++-------------- mdl/types/mapping_shape.go | 80 ++++++++++++++++++++ 2 files changed, 87 insertions(+), 57 deletions(-) create mode 100644 mdl/types/mapping_shape.go diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index 12b69d1b5..862ca927b 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -1993,41 +1993,13 @@ func (fb *flowBuilder) mappingRootIsList(im *model.ImportMapping) bool { // jsonStructureLookup is the one backend call deciding a JSON mapping's root // shape needs. -type jsonStructureLookup interface { - GetJsonStructureByQualifiedName(moduleName, name string) (*types.JsonStructure, error) -} +type jsonStructureLookup = types.JsonStructureLookup -// jsonMappingRootIsList answers mappingRootIsList's question from the two -// sources that are DEFINITE — the root element's JSON path, and the JSON -// structure the mapping is built on — and reports known=false when neither -// answers. The builder then falls back to the root element's occurrence bound; -// a check-time rule (MDL-MAP04) stays silent instead, since a guess there -// would be a refusal of something that may build. +// jsonMappingRootIsList answers mappingRootIsList's question from the sources +// that are definite; see types.JsonMappingRootIsList, shared with the lint +// rules so both decide the shape the same way. func jsonMappingRootIsList(lookup jsonStructureLookup, im *model.ImportMapping) (list, known bool) { - if im == nil { - return false, false - } - // A mapping rooted BELOW an array yields one object per item, whatever the - // structure's own root is (#267). The structure check below answers from - // js.Elements[0], which for `root choices/message` is the document root — - // an Object — so it reported a single object and mxbuild rejected the - // activity with CE0243 "the mapping ... now returns a value of type - // 'List of X'". The mapping document itself is right: Studio Pro's own - // OpenAI_API.IM_OpenAI stores the same MaxOccurs 1 on that root, so - // list-ness cannot be read off the root element either — only off its path. - if len(im.Elements) > 0 && im.Elements[0] != nil && - mappingRootPathCrossesArray(im.Elements[0].JsonPath) { - return true, true - } - if im.JsonStructure != "" && lookup != nil { - parts := strings.SplitN(im.JsonStructure, ".", 2) - if len(parts) == 2 { - if js, err := lookup.GetJsonStructureByQualifiedName(parts[0], parts[1]); err == nil && js != nil && len(js.Elements) > 0 { - return js.Elements[0].ElementType == "Array", true - } - } - } - return false, false + return types.JsonMappingRootIsList(lookup, im) } // addImportFromMappingAction adds an ImportXmlAction to the microflow. @@ -2212,31 +2184,9 @@ func (fb *flowBuilder) addExportToMappingAction(s *ast.ExportToMappingStmt) mode } // mappingRootPathCrossesArray reports whether a mapping root's stored JsonPath -// passes through an array on its way down. -// -// An array contributes a marker segment for its item — "(Object)" for objects, -// "(Wrapper)" for primitives — so any marker AFTER the first segment means the -// root sits inside an array and the mapping yields many objects: -// -// (Object) one object -// (Object)|data one object, a nested root -// (Array)|(Object) many — an array-rooted structure (#248) -// (Object)|choices|(Object)|message many — rooted below an array (#267) -// -// The first segment is skipped because it is always the document's own root -// marker, which says nothing about arrays. +// passes through an array; see types.MappingRootPathCrossesArray. func mappingRootPathCrossesArray(jsonPath string) bool { - segs := strings.Split(jsonPath, "|") - for i, seg := range segs { - if i == 0 { - continue - } - switch seg { - case "(Object)", "(Array)", "(Wrapper)": - return true - } - } - return false + return types.MappingRootPathCrossesArray(jsonPath) } // addSendEmailAction creates a SEND EMAIL activity (Microflows$SendEmailAction). diff --git a/mdl/types/mapping_shape.go b/mdl/types/mapping_shape.go new file mode 100644 index 000000000..5c1d1406c --- /dev/null +++ b/mdl/types/mapping_shape.go @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "strings" + + "github.com/mendixlabs/mxcli/model" +) + +// JsonStructureLookup is the one backend call deciding a JSON mapping's root +// shape needs. +type JsonStructureLookup interface { + GetJsonStructureByQualifiedName(moduleName, name string) (*JsonStructure, error) +} + +// JsonMappingRootIsList answers "does this import mapping return a list" from +// the two sources that are DEFINITE — the root element's JSON path, and the JSON +// structure the mapping is built on — and reports known=false when neither +// answers. The microflow builder then falls back to the root element's +// occurrence bound; a rule that judges an activity against the shape (MDL-MAP04, +// at check time and in lint) stays silent instead, since a guess there would +// be a report against something that may build and run. +// +// It lives here rather than in the executor so the lint rules, which cannot +// import the executor, decide the shape the same way the builder does. +func JsonMappingRootIsList(lookup JsonStructureLookup, im *model.ImportMapping) (list, known bool) { + if im == nil { + return false, false + } + // A mapping rooted BELOW an array yields one object per item, whatever the + // structure's own root is (#267). The structure check below answers from + // js.Elements[0], which for `root choices/message` is the document root — + // an Object — so it reported a single object and mxbuild rejected the + // activity with CE0243 "the mapping ... now returns a value of type + // 'List of X'". The mapping document itself is right: Studio Pro's own + // OpenAI_API.IM_OpenAI stores the same MaxOccurs 1 on that root, so + // list-ness cannot be read off the root element either — only off its path. + if len(im.Elements) > 0 && im.Elements[0] != nil && + MappingRootPathCrossesArray(im.Elements[0].JsonPath) { + return true, true + } + if im.JsonStructure != "" && lookup != nil { + parts := strings.SplitN(im.JsonStructure, ".", 2) + if len(parts) == 2 { + if js, err := lookup.GetJsonStructureByQualifiedName(parts[0], parts[1]); err == nil && js != nil && len(js.Elements) > 0 { + return js.Elements[0].ElementType == "Array", true + } + } + } + return false, false +} + +// MappingRootPathCrossesArray reports whether a mapping root's stored JsonPath +// passes through an array on its way down. +// +// An array contributes a marker segment for its item — "(Object)" for objects, +// "(Wrapper)" for primitives — so any marker AFTER the first segment means the +// root sits inside an array and the mapping yields many objects: +// +// (Object) one object +// (Object)|data one object, a nested root +// (Array)|(Object) many — an array-rooted structure (#248) +// (Object)|choices|(Object)|message many — rooted below an array (#267) +// +// The first segment is skipped because it is always the document's own root +// marker, which says nothing about arrays. +func MappingRootPathCrossesArray(jsonPath string) bool { + segs := strings.Split(jsonPath, "|") + for i, seg := range segs { + if i == 0 { + continue + } + switch seg { + case "(Object)", "(Array)", "(Wrapper)": + return true + } + } + return false +} From 4cf710d67f4fbc4eb4d52c1212d1e06eea7321f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 18:07:42 +0000 Subject: [PATCH 02/12] fix(check): report retrieve constraints comparing an attribute with a mistyped variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A retrieve constraint comparing an attribute with a variable of another type (`Kind = $Filter` with $Filter a String, `Email >= $Since` with a DateTime, `Visits = $Text` with a String) passed `check --references` and exec, then mx check reported CE0161 "Error(s) in XPath constraint." (mendixlabs/mxcli#1325). The reference pass now types each `attribute $var` comparison from the retrieved entity (script declaration or stored domain model) and the flow's parameters and declared variables. The compatibility table is measured on mxbuild 11.14.0 over 8 attribute x 8 variable types for `=` and `>=`: it is not type equality — a String variable against a DateTime attribute builds clean, Integer/Long/Decimal mix freely, and an enumeration needs the same enumeration. Untyped sides stay silent. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EsWAxrmEd7MBkay2zNpXNS --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../bug-tests/1325-retrieve-operand-types.mdl | 56 +++++ mdl/executor/validate.go | 6 + .../validate_retrieve_operand_types.go | 193 ++++++++++++++++++ .../validate_retrieve_operand_types_test.go | 104 ++++++++++ 5 files changed, 360 insertions(+) create mode 100644 mdl-examples/bug-tests/1325-retrieve-operand-types.mdl create mode 100644 mdl/executor/validate_retrieve_operand_types.go create mode 100644 mdl/executor/validate_retrieve_operand_types_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 935d6b987..299ffb52b 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -875,3 +875,4 @@ {"area": "mdl-executor", "date": "2026-10-06", "symptom": "`create constant M.Flag (Type: Boolean, DefaultValue: true)` (or `True`) stores DefaultValue \"true\". Studio Pro stores \"True\"/\"False\"; its constant dialog shows a stored \"true\" as False while the runtime reads it as true, so the developer sees one value and the app runs with another. `mx check` is silent and `describe constant` prints `true` for both, so only the stored unit shows it. Quoted `'True'` was the workaround. `alter settings constant @M.Flag value true` (or 'true') wrote the configuration override the same way", "cause": "createConstant rendered the AST literal with fmt.Sprintf(\"%v\"), so a Go bool became Go's lowercase \"true\"; the quoted string passed through verbatim, which is why only 'True' was right. The settings path stored the visitor's token text verbatim and never looked at the constant's type", "file": "`mdl/executor/cmd_constants.go` (storedConstantDefault, used by the create and create-or-modify branches, and by `alterSettingsConstant` in `mdl/executor/cmd_settings.go` via settingsConstantType); test `mdl/executor/cmd_constant_boolean_default_test.go`; bug-test `mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl`", "insight": "**`%v` on an AST literal is Go's spelling, not Mendix's**: any place that stringifies a parsed value into a stored property must normalise to the platform's form. The case-insensitive reader (formatDefaultValue's EqualFold) hid the writer's defect from describe, so a round-trip test could not catch it; assert on the value handed to the backend. Neither `mx check` nor the runtime distinguishes the two spellings — only Studio Pro's dialog does — so a build cannot verify this; the evidence is the reporter's Studio Pro 11.12.4 measurement plus the stored unit. Control: test written before the fix failed 5 of 6 spellings with `stored as \"true\"`/`\"false\"`, the quoted 'True' control passing; exec on a copy of testdata/pedapp then stores True/True/True/False. **Enumerate every write path for the value, not just the reported one**: the configuration override holds the same typed value and had the same defect; it needs the constant's type looked up (a String constant holding 'true' must stay lowercase). Control: the settings test failed 3 of 5 cases with `stored as \"true\"`/`\"FALSE\"` before the fix, the 'True' and String-constant controls passing; the stored Settings unit on a pedapp copy read 'true' for `value true` and `value 'true'` before, 'True' after, the String override 'true' both times", "refs": ["mendixlabs/mxcli#1321"]} {"area": "mdl/executor", "date": "2026-10-07", "symptom": "A button added to a snippet with `alter snippet … insert` (or replace) calling `call microflow M.F(P = $P)` with a snippet parameter fails mxbuild with CE0115 \"The arguments that are passed to microflow 'M.F' do not match the expected parameters and need to be refreshed\" on that button only; the same button in CREATE SNIPPET or via ALTER SNIPPET SET is clean", "cause": "`buildWidgetsFromAST` and `buildColumnSpecsFromAST` (mdl/executor/cmd_alter_page.go) seeded paramScope and localVariables from the mutator but not `isSnippet`, so `classifyFlowArgValue` returned kind \"parameter\" and the Forms$PageVariable named PageParameter instead of SnippetParameter", "file": "`mdl/executor/cmd_alter_page.go` (isSnippet from mutator.ContainerType() in both builders)", "insight": "**A pageBuilder built outside CREATE needs THREE scope fields, not two** — paramScope, localVariables and isSnippet; #1317 found the SET builder missing all three and this one missing the last. When adding a builder, copy the seeding from convertASTAction rather than from memory. **Unlike the Expression-form defect (#1140/#1317), mxbuild DOES catch the wrong slot** — CE0115 rather than CE1571 — so one script exercising CREATE, INSERT and SET on the same snippet with `mx check` is a complete A/B: only the broken path's button is named. Verified on 11.12.2: 1 error before, 0 after", "refs": ["mendixlabs/mxcli#1317", "mendixlabs/mxcli#1140"], "ce": ["CE0115"]} {"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $N = 1;` on a microflow/nanoflow/rule PARAMETER ($N: Integer or String) passes `mxcli check` and `exec`; mx check reports [CE7247] \"Parameter 'N' cannot be changed.\" at the Change variable activity", "cause": "Nothing modelled that a Change variable activity cannot target a parameter; the variable-kind tracking treated a primitive parameter like any declared primitive. Found while measuring MDL-SET01's object producers (#1323): the object parameter's CE7247 came back with a different wording, which was the tell that the restriction is on parameters, not on types", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable` primitive-parameter branch, `setTargetViolations`); rules via `validate_program.go` (check) and `rule_validation.go` `validateRuleSetTargets` (exec, `cmd_rules_create.go`); MDL-SET01 added to `execEnforcedMicroflowRules` in `validate.go`", "insight": "When one measurement returns a different MESSAGE under the same CE code, test the cause the message names — here 'Parameter … cannot be changed' fired for an Integer too, a whole adjacent class. Measure the neighbours before writing the rule: list parameters (`set` = Change list Replace, `add`) and member changes on parameters BUILD, so the rule is 'any non-list parameter', not 'any parameter'. Rules never run ValidateMicroflow: plain check needs a ValidateProgram hook and exec a gate in cmd_rules_create; putting it in validateRule instead would double-print under --references, which runs both", "refs": ["mendixlabs/mxcli#1323"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`retrieve … where Kind = $Filter` (Enumeration attribute, String variable), `Email >= $Since` (String vs DateTime), `Visits = $Text` (Integer vs String) pass `check --references` and `exec`; mx check reports [CE0161] \"Error(s) in XPath constraint.\" at Retrieve object(s) activity (mendixlabs/mxcli#1325)", "cause": "The --references pass resolved a constraint's members against the entity (validateRetrieveMembers, #1213) but never compared the member's type with the right-hand variable's declared type", "file": "`mdl/executor/validate_retrieve_operand_types.go` (`validateRetrieveOperandTypes`, `xpathOperandTypesCompatible`), wired beside `validateFlowBodyReferences` for microflows and nanoflows in `validate.go`", "insight": "Measure the whole matrix before writing the rule — one mx check over 8 attribute types x 8 variable types x {=, >=} with a unique `@caption` per retrieve (CE0161 names only the activity caption, not the microflow). The result is NOT \"types must be equal\": a String variable against a DateTime attribute builds clean, Integer/Long/Decimal compare freely, an enumeration needs the SAME enumeration. Separately measured but out of scope: `>=` on a Boolean or Enumeration attribute is CE0161 even against a same-typed variable. The flow collector has no parameter list, so the check takes `s.Parameters` from the create statement; variable types = params + `declare`, anything else (objects, lists, loop iterators) stays silent.", "refs": ["mendixlabs/mxcli#1325", "mendixlabs/mxcli#1213", "mendixlabs/mxcli#176"], "ce": ["CE0161"], "rules": []} diff --git a/mdl-examples/bug-tests/1325-retrieve-operand-types.mdl b/mdl-examples/bug-tests/1325-retrieve-operand-types.mdl new file mode 100644 index 000000000..c9e08239a --- /dev/null +++ b/mdl-examples/bug-tests/1325-retrieve-operand-types.mdl @@ -0,0 +1,56 @@ +mdl 1; +-- mendixlabs/mxcli#1325 — a retrieve constraint comparing an attribute with a +-- variable of another type passed `check --references` and exec, then mx check +-- reported [CE0161] "Error(s) in XPath constraint." at the retrieve activity. +-- +-- Measured on mxbuild 11.14.0, `=` and `>=` alike. These are rejected, and +-- `mxcli check