Fix three upstream check/exec gaps (#831, #832, #833) + two corruptions found while verifying - #115
Merged
Merged
Conversation
`create non-persistent entity X ( Name: String(100) not null error '…' )`
passed both `mxcli check` and `mxcli exec`, and only a real build caught it:
[error] [CE0070] "Validations rules are not allowed on entity 'X',
because it is not persistable."
`not null` and `unique` ARE validation rules — Studio Pro models "required"
and "uniqueness" as rules on the entity rather than as column constraints — so
Mendix rejects both on a non-persistable entity. Nothing in mxcli connected the
attribute constraint to the entity's persistence kind.
The construct matrix was established against mxbuild 11.6.6 rather than taken
from the issue text: `not null` with a message, `not null` bare, and `unique`
each produce CE0070, while a plain attribute does not. The bare form matters —
the report only showed the message form, and treating the message as the
trigger would have left half the bug in place.
Scoped to the CREATE path, where the persistence kind is known. An
`ALTER ENTITY … ADD ATTRIBUTE` does not carry it and cannot be told apart from
a persistent entity without a project — the same limitation MDL020 has, and the
rule comment says so rather than pretending otherwise.
Checked for false positives before committing: no file in mdl-examples/ trips
the new rule, and scripts/check-skill-mdl.sh still passes all 189 checkable
blocks. The negative test is a .fail.mdl (must fail check, enforced by
`make check-mdl`), paired with an -ok.mdl that pins the other edge — the same
constraints on a persistent entity, which mxbuild confirms builds clean.
Fixes mendixlabs#832
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
A statement's own name is "defined in the script but not yet created" at the
moment it fails, so annotateForwardRef matched it and appended
hint: X is defined later in this script — move its create statement
before this one
to any validation error whose message named its own subject. The advice is
impossible to follow: the statement it points at is the one that failed.
Surfaced by MDL054, whose message names the entity being created, but the
misfire is general — it applies to any create statement whose error mentions
itself.
The fix uses the ast.Statement parameter the function already took and
deliberately ignored (`_ ast.Statement`): collect the names the failing
statement defines and skip them. A genuine forward reference — a name some
LATER statement defines — is still annotated, which the test pins alongside
the regression.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
`retrieve $L from Mod.Entity where [Name = $RefProduct/Mod.Product_Category/Name]`
passed both `mxcli check` and `mxcli exec`, and the build then failed with
CE0161 "Error(s) in XPath constraint". Mendix XPath reaches at most one hop off
a variable, and nothing checked the hop count.
The valid/invalid boundary was established against mxbuild 11.6.6 rather than
inferred, and it is narrower than it first appears:
$Var/Attr VALID the parameter's own attribute
$Var/Mod.Assoc VALID one hop, the associated object
$Var/Mod.Assoc/Attr CE0161 two or more hops
so the rule keys on the number of segments. The obvious formulation — flag a
module-qualified segment following a variable — would have rejected the middle
form, which builds clean. Confirmed by building all three and then dropping the
offender to verify the remaining two report 0 errors.
This is a rejection rather than a smarter serializer because there is no valid
XPath for the two-hop form: the constraint has to be restructured, and only the
author knows which of the two shapes they meant.
Both rewrites the message recommends were built and confirmed at 0 errors
before the text claimed they work — retrieving the associated object first
(one hop is a legal retrieve SOURCE) and constraining on that variable's own
attribute, or inverting so the traversal starts at the entity being retrieved.
No positive example in mdl-examples/ trips the rule, and
scripts/check-skill-mdl.sh still passes all 189 checkable blocks. Negative test
is a .fail.mdl paired with an -ok.mdl carrying both rewrites plus the two
one-hop forms that must not be flagged.
Fixes mendixlabs#831
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
…labs#833) `mxcli check` reported MDL048 for `where [id = $GuidText]` and `mxcli exec` wrote the microflow anyway, so a script that skips check produced a project the build fails with CE0161. The cause is two validators: the exec path ran ValidateMicroflowBody (semantic errors), while the MDL0xx rule set lives in ValidateMicroflow, wired only into cmd_check.go and the LSP. Same shape as mendixlabs#836, where a guard existed on every exec path but was never reached from validate. Enforced at exec via an explicit allowlist of rules whose claims were verified against mxbuild 11.6.6 — MDL047, MDL048 and MDL055, all XPath-constraint rules whose constructs were built and confirmed to fail CE0161. Blanket promotion of all 17 error-severity rules was implemented first and then reverted, because it makes every rule a write barrier and at least one rule is wrong: MDL009 ("enumeration splits require exactly one value per branch") is a FALSE POSITIVE — a multi-value branch covering every enum value builds at 0 errors, and the shipped write-microflows skill documents exactly that form. It also broke an existing test whose fixture uses `else` on an enum split. MDL008 is by contrast correct (mxbuild reports CE0079 per uncovered value plus CE0773), which is the point: the two rules look alike and only a real build tells them apart. A test now fails if the allowlist is widened without that check. Placed in the create handler rather than validateWithContext, so `check --references` does not report each violation twice. Warnings are never promoted — check itself passes with them. Fixes mendixlabs#833 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
MDL009 errored on `when Open, Pending then` with "Mendix enumeration splits require exactly one value per branch". Mendix does no such thing: verified on mxbuild 11.6.6, a multi-value branch covering every value plus `(empty)` builds with 0 errors. So `mxcli check` was rejecting valid MDL — and contradicting the shipped write-microflows skill, which documents that very form. The rule was found while deciding what mendixlabs#833 could safely promote to a hard exec failure. What actually fails the build is a MISSING branch. An enum split is an exclusive split needing one outgoing flow per condition value, and an uncovered one is CE0079 "The 'X' condition value should be configured in properties for an outgoing flow." MDL056 checks the `(empty)` branch specifically. That half of CE0079 is universal and needs no knowledge of the enumeration's members — confirmed it fires even when the split is on a `not null` enum attribute — so it works from the statement alone. Full value coverage is deliberately left out: it requires resolving the split variable's type to an enum member list, which ValidateMicroflow cannot see, and guessing would trade one false positive for another. A new rule ID rather than a repurposed MDL009, so anything still citing the old number keeps meaning the old, wrong thing. MDL008 (no `else` branch) is correct and stays — mxbuild reports CE0079 for each uncovered value AND CE0773 on the else flow, so an `else` does not stand in for the missing flows. The skill's CASE example used `else` and is corrected here in the same change, since leaving it would teach MDL that fails the build. No positive example in mdl-examples/ trips MDL056; check-mdl and check-skill-mdl both pass. The -ok.mdl repro builds at 0 errors in mxbuild, including the multi-value branch MDL009 used to reject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
`split type` produced a project mxbuild could not load at all:
KeyNotFoundException: The given key '<guid>' was not present in the
dictionary at StreamingBsonUnitReader.ResolvePostponedProperties()
`mxcli check` passed and `mxcli exec` reported success. Reproduced on Mendix
11.6.6 and 11.13.0. Found while testing whether an enum-split `else` is
version-dependent.
Two gaps in the modelsdk writer, both the mendixlabs#791 shape — an object dropped at
serialization while the sequence flows pointing at it are still written:
1. microflowObjectToGen had no *microflows.InheritanceSplit case, so the
split hit `default: return nil` and vanished. Three flows referenced its
$ID; that is the dangling pointer the loader trips on.
2. caseValueToGen had no InheritanceCase case, so every branch degraded to a
bare Microflows$NoCase and lost the entity it selects on. Its
value-receiver normalisation omitted the type as well, so handling only
the pointer form would still have missed half the calls.
Diagnosed with the recipe the symptom table already records for this class:
dump the microflow, collect every $ID, check each key ending in `Pointer`
resolves. Before: 27 objects, 10 pointers, 3 dangling. After: 28, 10, 0.
Field list taken from the generated type rather than the legacy serializer.
Legacy writes ErrorHandlingType on the split, but initInheritanceSplit has no
such property — Mendix does not define it there — so the codec omits it.
Verified end-to-end: the repro script now reports 0 errors on both 11.6.6 and
11.13.0, where it previously could not be loaded.
Two modelling rules were confirmed on both versions along the way and are
recorded in the repro: a type split needs an outgoing flow for every type
INCLUDING the base entity (CE0090 otherwise), and an `else` does not substitute
for the base-type case.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
The write-microflows skill taught `case Spec` + `else` with no branch for the
base entity, and described `else` as handling "objects that do not match any
listed specialization". It does not. An object-type decision needs an outgoing
flow for every listed type, and without the base entity the build fails
[error] [CE0090] "The 'X' value should be configured for an outgoing flow."
`else` IS accepted — it serializes as Microflows$NoCase — which is what made
the guidance look right. It simply does not satisfy coverage, so it is
redundant once every type has a branch.
Matrix verified on 11.6.6 and 11.13.0:
specializations + base 0 errors
specializations + base + else 0 errors (else redundant)
specializations + else only CE0090
The examples also omitted a return after `end split;`. Branch bodies converge
on a merge that continues to the microflow's end event, so a non-void microflow
needs one — otherwise mxcli check reports MDL003 and the build fails CE0067
"The 'Return value' property is required."
Two shipped examples had the same defect and did not build:
mdl-examples/bug-tests/365-microflow-inheritance-split.mdl and
475-inheritance-split-continuing-branch-merge.mdl — the latter's own header
claimed "mx check against the resulting MPR reports 0 errors", which had not
been true. Both now build clean on 11.6.6 and 11.13.0.
475's added base case is deliberately a TERMINATING branch. The scenario it
pins is "exactly one non-split branch continues"; an empty, falling-through
body would make two branches continue and quietly retire the regression.
Verified after the edit that the post-split activity still renders outside both
case bodies and that the describe→exec roundtrip is mxbuild-clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes upstream issues #831, #832 and #833. Verifying them against real mxbuild turned up three further defects, each committed separately.
Every claim below was checked against a real build — mxbuild 11.6.6 throughout, and 10.24.24.119349 + 11.13.0 where a version question arose. No rule was added on reasoning alone.
The three issues
mendixlabs#832 — validation rules on non-persistent entities (
MDL054, new)create non-persistent entity X ( Name: String(100) not null error '…' )passedcheckandexec; only a real build caught it withCE0070 "Validations rules are not allowed on entity 'X', because it is not persistable."not nullanduniqueare validation rules — Studio Pro models "required" and "uniqueness" as rules on the entity, not column constraints.The construct matrix came from mxbuild, not the issue text: bare
not nullproduces CE0070 too. The report only showed the message form, and treating the message as the trigger would have left half the bug in place.Scoped to CREATE, where the persistence kind is known.
ALTER … ADD ATTRIBUTEdoesn't carry it — the same limitation MDL020 has, stated in the rule comment rather than papered over.mendixlabs#831 — XPath traversal from a variable (
MDL055, new)where [Name = $RefProduct/Mod.Product_Category/Name]passed both, then failedCE0161. The boundary is narrower than it looks:$Var/Attr$Var/Mod.Assoc$Var/Mod.Assoc/AttrThe obvious rule — flag a qualified segment after a variable — would reject the middle form, which builds clean. The rule keys on hop count instead. Both rewrites the error message recommends were built and confirmed at 0 errors before the text claimed they work.
mendixlabs#833 —
execdidn't enforce whatcheckrejectsNot really about MDL048. There are two validators: the exec path ran
ValidateMicroflowBody, while the MDL0xx rule set lives inValidateMicroflow, wired only intocmd_check.goand the LSP. All 17 error-severity rules were check-only, so a script that skippedcheckwrote microflows the build rejects. Same shape as mendixlabs#836.Enforced via an explicit allowlist of rules verified against mxbuild (MDL047/048/055). Blanket promotion was implemented first and reverted — see below.
Three defects found while verifying
MDL009was a false positive. It errored onwhen Open, Pending then, claiming Mendix requires one value per branch. A multi-value branch covering every value plus(empty)builds at 0 errors, and our ownwrite-microflowsskill documents that form — socheckwas rejecting valid MDL. This is why mendixlabs#833 promotes an allowlist rather than the whole set: blanket promotion would have madeexecrefuse valid MDL. Retired and replaced withMDL056, which checks what actually fails — the missing(empty)branch (CE0079). That half is universal (it holds even on anot nullattribute), so it needs no enum lookup. Full value coverage is deliberately left out: it needs the enum's member list, and guessing would trade one false positive for another.MDL008(noelseon an enum split) is correct and stays — mxbuild gives CE0079 per uncovered value and CE0773. That MDL008 and MDL009 are indistinguishable by inspection is precisely why the allowlist requires a build.split typeproduced an unloadable project.mxcli check✓,exec✓, then mxbuild died before validating anything:KeyNotFoundException … at StreamingBsonUnitReader.ResolvePostponedProperties. Reproduced on 11.6.6 and 11.13.0. Two gaps, both the mendixlabs#791 shape:microflowObjectToGenhad noInheritanceSplitcase (the split was dropped while three flows kept pointing at its$ID), andcaseValueToGenhad noInheritanceCasecase (every branch degraded to a bareNoCase). Diagnosed with the recipe already in the symptom table — dump, collect every$ID, check each*Pointerresolves: 27 objects / 3 dangling → 28 / 0.Field list taken from the generated type, not the legacy serializer: legacy writes
ErrorHandlingTypeon the split, butinitInheritanceSplithas no such property.Type-split docs taught a shape that fails the build. The skill paired
case Specwithelseand saidelsehandles unmatched objects. It doesn't —elseis accepted (it serializes asNoCase) but doesn't satisfy coverage, so the base entity needs its own case or the build fails CE0090. Two shipped examples had the same defect and did not build; 475's header even claimed "mx check reports 0 errors". Both build clean now on 11.6.6 and 11.13.0.475's added base case is deliberately a terminating branch: the scenario it pins is "exactly one non-split branch continues", and a falling-through body would make two branches continue and silently retire the regression.
Also fixed
A validation error that named its own subject got the hint "X is defined later in this script — move its create statement before this one" — advice impossible to follow, since the statement it points at is the one that failed.
annotateForwardReftook anast.Statementit deliberately ignored; it now uses it.Verification
go test ./...green;make check-mdlandscripts/check-skill-mdl.sh(189 blocks) both pass.mdl-examples/for false positives — zero..fail.mdlnegative test (must failcheck, enforced bymake check-mdl) paired with an-ok.mdlpinning the other edge, and each-ok.mdlwas built with mxbuild at 0 errors.TestValidateMicroflowRules_UnverifiedRulesNotPromotedfails if the exec allowlist is widened without a build check.Notes for review
execnow refuses three constructs it previously wrote. Intended — they all fail the build — but it is a behaviour change for scripts that never rancheck.count(…)/empty(…)in widget conditionals: unrelated to this PR's rules, but worth knowing they are not client-expression functions and surface as CE0117.make check-mdlonly runscheck, never a build, which is how three pieces of reference material shipped a shape that fails mxbuild. A build-backed example check would close it — flagged rather than changed here.describeemits an emptyelseblock on a type split that was never authored. It re-parses and builds clean.🤖 Generated with Claude Code
https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
Generated by Claude Code