Sync ako/mxcli: page round-trip fidelity, check/exec agreement, domain-model and security fixes - #1146
Conversation
…he source
`alter page … { set DataSource = DATABASE Mod.Entity on dvCust; }` passed
`check`, printed `Altered page …` with exit 0, and left the DataView with no
usable datasource: `describe page` rendered the widget with the property gone,
and the only other signal was CE7007 from mxbuild — naming the widget, never
the statement that broke it.
One mapping stood in for several. A DATABASE source has no single stored shape;
the widget holding it decides which element Studio Pro writes
(Forms$ListViewXPathSource on a list view, CustomWidgets$CustomWidgetXPathSource
on a pluggable widget, Forms$GridXPathSource on a grid). A data view has no
database form at all — it binds to one object — which is why CREATE PAGE's
dataViewSourceToGen already refused that pairing while SET wrote it silently.
serializeDataSourceBson instead emitted a Forms$DataViewSource (the "data from
context" source) with the entity in EntityRef and SourceVariable left null,
which is neither shape and is why the describe reader, needing a SourceVariable,
rendered nothing.
The setter now refuses a database source, dispatching on the stored widget's
$Type for the remedy: a data view is told which sources it can take (REPLACE
would be a dead end there — CREATE PAGE refuses the same pairing), every other
widget is pointed at REPLACE, which reaches the real builder. Rebuilding the
shapes in the mutator would be a second copy of listViewSourceToGen in raw BSON,
the duplicate-resolver drift CLAUDE.md warns about.
The refusal lives once, in the mutator, so `check -p --references` — which
dry-runs the setter against a pagemutator.Probe() copy — and `exec` cannot
disagree.
Measured on two copies of a real Mendix 11.13.0 app:
with the fault check --references -> "Check passed!"
exec -> "Altered page …", exit 0
describe page -> dataview dvCust { … } (no source)
mx check -> 1 error, CE7007 at Data view 'dvCust'
with the fix check --references -> refused, exit 1
exec -> refused, exit 1
describe page -> datasource unchanged
mx check -> 0 errors
Control: with the fix reverted, both new tests fail with the reported symptom
(the statement accepted; check reporting 0 errors), while the non-database
retypes mendixlabs#855 added keep passing.
The alter-page skill advertised the database form as supported; corrected, along
with the `mxcli syntax page.alter` help.
upstream mendixlabs#1032
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSDurGmL9HgDdLhpMqZt6o
…pplies Two halves of one concern: mxcli should not quietly help you author a widget your app cannot build. #518 and #538 made `staticimage` and `dynamicimage` round-trip, which was right — a project being converted up already contains them, and the pre-fix describe -> exec deleted them. But the same work made both materially easier to author (Image:, DataSource:, DefaultImage:, thumbnail, enlarge all newly reachable) while nothing warned the author. Measured: no validator mentions either widget, so the only signal was CE0582 at the far end of a build. MPR012 reports them. The linter and not `mxcli check`, deliberately: `check` validates a script, and describe -> exec of a legacy page is a legitimate lossless operation that a warning would flag every time — a rule that fires on correct work is noise. `lint` audits the project, where "this page holds a widget your client cannot render" is wanted once. The marketplace exclusion comes free and is the part worth measuring: ctx.Widgets() already filters any module with a Source, so the rule never fires on the Studio Pro static images a blank app inherits from FeedbackModule — content the reader cannot fix and an update would replace. Measured on a blank 11.12.1 app carrying both widget kinds: 8 legacy image widgets in the BSON, 7 indexed by the catalog, 5 in the user's own module, and lint reported exactly those 5. A deny-list of exactly two storage names, never an allow-list: the one widget such a rule must never fire on is the pluggable Image — the replacement it recommends. Also corrects a version claim carried in three places, including two I wrote. The reference guide says the React client was added in **10.7**: "The Dynamic Image widget, which is not supported by the React client added to Mendix in 10.7, can be converted to an Image widget through the context menu of the widget when the React client is enabled." So CE0582 fires on 10.7+ wherever that client is enabled, not "in the Mendix 11 React client". A version boundary copied from a sibling comment rather than from the vendor doc is the same class of error as a floor copied from a proposal's sample output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016JgBheMTV6UiQstQ2nLyay
describe page → exec over a Studio Pro-authored page reported "Replaced", not "Unchanged", so ADR-0008's elision could not fire and the unit churned in version control on every re-run. mx check was 0 errors either way. Measured on ako/TestApp's Rules.RuleAction_NewEdit at 11.14.0: fourteen differences, in four independent classes, all "the rebuild writes a constant where Studio Pro stores a value". 1. Page header. pageToGen hardcoded Autofocus, CanvasWidth and CanvasHeight. Studio Pro varies all three per page — CanvasWidth takes seven distinct values across those 67 pages and the hardcoded 1200 matched four, so a round trip moved the canvas of the other 63. Now carried from the stored document on a rewrite; a new page, which has no stored document, still gets the defaults. 2. Client-action defaults. save_changes, cancel_changes, close_page and delete_object never wrote DisabledDuringExecution, which Studio Pro stores true on all 39 of them; save_changes wrote SyncAutomatically true where all 8 store false. 3. AttributeRef.EntityRef, present on 338 of 338 stored refs (313 null, 25 navigated), was emitted only on the navigated branch. 4. Forms$PageVariable: only the field carrying a value was set, so the other five keys were never marked dirty and the encoder omitted them. 3 and 4 go in the codec's TypeDefaults rather than at each construction site — PageVariable is built in three places — which needed one new kind, FalseFields, since a bool's zero value is never dirty. Result on that page: 14 differences → 1. The remaining one is a pluggable widget Object property, which is the CE0463 subsystem and needs its own investigation. Two things worth knowing, both learned the hard way here: mxcli round-tripping its own output proves nothing about this class. Measured: the MDL bug-test reports "Unchanged" on the unfixed build too, because mxcli writes the page and mxcli describes it. The reference has to be a Studio Pro document, which also limits the committed-fixture idea in the issue. The detecting evidence is the Go tests, each run against a stubbed-out fix. The first version of the header carry used a .(int32) assertion — the natural one, since the gen setter takes int32 — and matched nothing, because Studio Pro stores both dimensions as int64. Its unit test passed regardless, since the fixture wrote int32: the test encoded the assumption under test. The read is now width-agnostic and the test asserts both. Closes #541 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
Follow-up to mendixlabs#1121, which fixed this defect in a page's PARAMETERS and left the header out of scope. Measured against the Mendix Model SDK's StructureVersionInfo records (mendixmodelsdk 4.115.0): Page.Autofocus 11.1.0 Page.Variables 10.17.0 Snippet.Variables 10.17.0 All three were written unconditionally, so every page and snippet mxcli created for a Mendix 10 project carried a key that project's metamodel does not declare — the class that makes Studio Pro throw InvalidOperationException at MprProperty.cs. mxbuild does not catch it: measured on 10.24.25, 0 errors with the key present and 0 errors without it. They arrive by two different routes, and only one is a gen property. Autofocus is set through gen, so it is simply not set below 11.1. Variables exists only because the codec's Studio Pro defaults registry emits it as an empty typed-array marker, and a gen PartList has no "present but empty" state to leave unset — so there is nothing to skip and the suppression has to be in the encoder. That registry is global and keyed by $Type alone, so it cannot see a project version: hence Encoder.OmitKeys, per-encode. The zero Encoder suppresses nothing, so every other caller is unaffected. Suppressing a key and dropping data are different things. The empty Variables marker is safe to suppress; variables the script DECLARED are refused instead, naming them and the floor (guard-don't-drop, ADR-0005) — silently dropping them leaves widgets referencing names that are gone (CE1151) from a statement that reported success. CreatePage/UpdatePage and CreateSnippet/UpdateSnippet now share encodePage / encodeSnippet, so the guard cannot be applied on one path and forgotten on the other. ALTER PAGE needs nothing: the page mutator edits the stored raw BSON and marshals it back, so it can never invent a key. Verified on a real 10.24.25 project created with `mxcli new`. The page written by this build has no Autofocus and keeps Variables (10.24 >= 10.17); the snippet keeps Variables; `mx check` is 0 errors. The control is the same script run by a pre-fix binary against a copy of that project, which writes Autofocus — the only difference between the two documents. Unit tests carry a control too: with both guards stubbed to the pre-fix behaviour they report the key emitted at 9.24, 10.24.25 and unknown-version. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aHj6mJwKCZD7EX7wcD6jW
fix(alter page): refuse SET DataSource = DATABASE instead of wiping the source
fix(lint): report the legacy image widgets, and correct when CE0582 applies
Stop a page rewrite moving state nobody asked to change (#541)
… three more
`describe page` → `exec` over a Studio Pro page silently dropped six things,
with mx check at 0 errors on both sides. The one that matters:
…/Widgets/[1]/IsPasswordBox True → False
a password field round-trips into a plaintext text box. CLAUDE.md makes
describe → rename → exec the copy operation, so copying a login or
change-password page lost it silently.
Measured on ako/TestApp's Administration.ChangePasswordForm at Mendix
11.14.0. Four different causes behind one symptom, which is why triage came
before any code:
- IsPasswordBox — the model and writer carried it; nothing parsed it and
nothing emitted it.
- Validation — widgetValidationToGen() wrote a default EMPTY
Forms$WidgetValidation over whatever was stored, on five widget types.
- ReadOnlyStyle — wired for CheckBox only. A DataView's draws no
MDL-WIDGET07 warning because staticWidgetKnownProps is deliberately a
union across widget types, so it passed check and was dropped anyway.
- PopupCloseAction — pageToGen wrote "" unconditionally.
Plus two typed-array markers: ParameterMappings is marker 2 on 220 of 220
stored lists in every parent type, and OutputMappings is present on 91 of
91 MicroflowSettings. An empty list needs MandatoryListMarkers, since
RegisterListMarker keys on a child element that is not there.
Three things measured rather than assumed, each of which would have been
wrong the obvious way:
A DataView's ReadOnlyStyle default is Control (47 of 56, never Inherit),
not the Inherit every other input widget uses.
The validation expression is emitted QUOTED, not bracketed. `[...]` is the
XPath-constraint spelling and propertyValueV3 parses it as an array, so the
builder saw []any and GetStringProp yielded "" — the emitter's own unit
test was green while the real round trip still lost the value.
PopupCloseAction is deliberately not carried from the stored document the
way the canvas properties are: it names a widget, and a rewrite rebuilds
the tree from the statement, so a carried name could dangle. DESCRIBE emits
it instead.
Result on that page: 17 differences → 9, and all 9 remaining are #549, a
separate carry problem. Verified at the artifact level — same project, same
script, only the binary differing: the pre-fix build turns both stored
password boxes into plaintext ones, the fixed build preserves them, and
mx check is 0 errors after round-tripping four pages.
Closes #550
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
CodeQL's clear-text-logging query read `w["IsPasswordBox"]` as a lookup of a
credential and followed the boolean, field-insensitively, into every error the
page writer can return — surfacing as a high-severity alert at an unrelated
example's `fmt.Printf("Error creating page: %v\n", err)`. The value is a
design-time flag ("render this text box as a password field") and the logged
expression is an error, so the classification is wrong; the alert is new with
this branch because the inline map index is (PR #551 is the first to read that
key here — the same CodeQL check was clean on #542 and #546).
Reading the key through a helper takes the sensitive-looking literal out of the
index position. No behaviour change: the describe/builder round-trip tests for
Password are unchanged and pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
This reverts 3ac6df5. The hypothesis behind it — that CodeQL read the inline `w["IsPasswordBox"]` map index as a credential lookup — was wrong: the re-run produced a byte-identical alert, and running the query locally shows the source is `modelsdk/gen/pages/types.go:33658`, the `o.isPasswordBox` property descriptor in the generated SetProperties slice. The alert reproduces on unmodified main (codeql 2.27.0, Security/CWE-312/CleartextLogging.ql, 6 results including this exact one), so it is not this branch's to fix, and the accessor bought nothing. Keeping the PR scoped to #550. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
Conflict in page_write.go's UpdatePage: main's #541 added carryStoredPageHeader (carry Autofocus/CanvasWidth/CanvasHeight off the stored unit so a rewrite does not move state nobody asked to change), while this branch routed both write paths through encodePage for the version guards. Both are wanted, so encodePage grew a carry hook: CreatePage passes nil (a new page has no stored document), UpdatePage passes the carry. The merge also created a case neither side had alone. A pre-fix mxcli wrote Autofocus into Mendix 10 projects, so on such a project the STORED value is itself the defect — main's carry would faithfully preserve a key that project's metamodel does not declare, making the repair a no-op. The carry is now gated on the same floor: below 11.1 the stored value is dropped rather than carried. That is repair, not data loss — there is no version of the property the project can express. carryStoredPageHeader takes the project version explicitly rather than reading it off the backend, so the decision is testable against the 11.6.6 fixture with a synthetic 10.x version. Verified on the real 10.24.25 project from this branch's earlier work, whose page was written by a pre-fix binary and therefore carries Autofocus: after a rewrite with the merged build the key is gone, Variables is kept, and mx check is 0 errors. The new test carries a control — with the carry guard stubbed it reports the key carried at 10.24.25, 11.0 and unknown-version — and asserts the carry still works at and above the floor, so a failure below it cannot be "the carry never works". main's own header tests pass unchanged: their fixture is 11.6.6, above both floors, so a new page there still gets Autofocus and the stored value is still carried. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aHj6mJwKCZD7EX7wcD6jW
fix(pages): stop writing 11.1/10.17 header keys into older projects
Stop describe losing a password field, its validation and three more (#550)
…eates
MDL-PAGEARG01 refused
datagrid dgRequests (DataSource: DATABASE M.ServiceRequest,
onClick: SHOW_PAGE M.Edit(ServiceRequest: $currentObject))
with "widget `dgRequests` is not inside a data view, list view or grid row".
A list widget's onClick fires PER ROW, so the row it renders IS the context
object — and on a `listview` the refusal contradicted its own wording. Since
exec refuses a script whose check reports an error, this was a blocker rather
than a warning: the reporting project could not apply the slice at all.
The mendixlabs#1029 guard judged every widget's own action in the context its PARENT
supplies. That is right for a button and wrong for the widget that establishes
the context. argContextForOwnAction draws the line where it belongs: a widget
binding a source of its own supplies the context for its own action. A source
in a shape the pass cannot read degrades to UNKNOWN, so the guard stands down
rather than refusing what it cannot prove is discarded.
Measured on mxbuild 11.14.0, in one fresh app, with the actions verified to be
stored (describe page) so the zero is not a dropped action:
datagrid + DATABASE source + onClick($currentObject) 0 errors
listview + DATABASE source + onClick($currentObject) 0 errors
Controls, all still refused: a foreign variable on a row action, mendixlabs#1029's
page-level button, and a button standing beside the grid rather than in it.
1029-showpage-arg-without-context.fail.mdl still exits 1; the two valid mendixlabs#1029
and #295 bug-tests still pass. Before the fix the new unit test fails with the
reported message verbatim.
The same mxbuild run exposed a separate defect, filed as #576 and deliberately
NOT written into the bug-test as a passing case: `DataSource: Mod.Car` on a
datagrid is silently dropped, so that widget is CE0488 plus a real CE1571.
Closes #552
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
fix(pages): judge a list widget's own row action in the context it creates
…y end at
DROP ENTITY swept only the regular Associations collection, so every
CROSS-MODULE association pointing at the deleted entity survived. Dropping the
local BY-ID (FROM) end left a 16-byte pointer to an element that no longer
exists, and mxbuild 11.14.0 could then not LOAD the project:
ERROR: System.AggregateException … (The given key
'49751a65-d5f9-456c-887e-3f14bacb8822' was not present in the dictionary.)
at StreamingBsonUnitReader.ResolvePostponedProperties()
No CE code and no document named — the failure is in the storage layer, above
the consistency checker, so it reads as "the project is corrupt". Dropping the
BY-NAME (TO) end is milder and still wrong: CE1613 at the cross-module
association.
removeCrossAssocsReferencing matches both ends, because a cross-module
association addresses them differently: FROM by element id (local to this
domain model), TO by qualified name (it lives in another module). Called from
DeleteEntity locally and in its cascade over the other domain models.
Reported against a VIEW entity, whose associations are derived from its OQL so
there is no CREATE ASSOCIATION to undo. Nothing here is view-entity specific:
the first probe — view entity and source entity in the SAME module — did not
reproduce, and that negative is what identified cross-module as the variable. A
plain `create association A.X from A.X to B.Y` plus `drop entity A.X`
reproduces the identical crash.
Measured on mxbuild 11.14.0, one project carrying all three shapes:
before drop by-id end project does not load (exception above)
drop by-name end CE1613 at the cross-module association
after all three drops project loads, 0 errors
Controls: an untouched cross-module association survives both deletes (in the
unit test and in the bug-test), and the single-module case still works. Before
the fix the new test fails in both directions with the orphan left behind.
Closes #553
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
An access rule on an entity carrying AutoOwner or AutoChangedBy made mxbuild
report the whole module as CE0066 "Entity access is out of date" — and
UPDATE SECURITY, the documented repair for that error, printed "Reconciled 1
access rule(s)" and left it standing, because it re-added the entry that caused
it. Four lines on a clean production-security app reproduce it:
alter entity Mod.Fab add attribute Owner: AutoOwner;
update security;
mxcli wrote a MemberAccess for the implicit System.owner / System.changedBy
association. Mendix maintains those members itself and treats a rule naming one
as out of date. Measured on mxbuild 11.14.0, one entity, one rule, one variable
at a time:
AutoOwner + MemberAccess System.owner CE0066
AutoOwner + no entry 0 errors
AutoChangedBy + MemberAccess System.changedBy CE0066
AutoChangedBy + no entry 0 errors
So all four audit members follow one rule. The DATE half was already right
(issuetracker #20); the association half was assumed to be the opposite case
because Mendix really does add those two implicitly — the same inference the
earlier finding warned against in this very file ("ask mxbuild what it wants
instead of inferring symmetry").
Three parts, because two writers had to agree and a damaged project has to be
repairable:
- the GRANT handler no longer adds the entry;
- ReconcileMemberAccesses no longer adds it;
- ReconcileMemberAccesses REMOVES a stored one, ahead of the foreign-module
branch that would otherwise preserve it forever on the grounds that System
is not loaded here. Without this, `update security` still could not repair
what it exists to repair. Measured: CE0066 -> 0 errors on both damaged
projects. It also clears a stale System.owner left behind when the flag is
turned off again.
Exactly System.owner and System.changedBy, not every System.* reference: an
entity specialising a System entity legitimately inherits that module's real
associations.
Control: with the fix reverted the new test fails on all three flag
combinations, naming the entry and the CE code; the bug-test's audit-free
entity keeps its own attribute's member access, so a build writing no members
at all fails it too.
Closes #554
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2aYb3zDscezm874CDTH6R
fix(domain model): drop cross-module associations with the entity they end at
fix(security): write no member access for the audit associations
AI Code ReviewI need to review this pull request against the provided checklist. Let me analyze the PR content and check it against each section of the checklist. First, let's understand what this PR does based on the description: The PR contains ten commits from ako/mxcli:main that are not yet upstream. The body describes several issues that were fixed: Severe Issues
Page round-trip fidelity
|
Ten commits from
ako/mxcli:mainthat are not yet upstream. Each carries itsmeasurement in the commit body; the grouping below is by what was wrong, not by
file.
Two of these destroy or silently alter user work and are worth taking first.
Severe
A dropped entity could leave a project that will not LOAD.
DROP ENTITYswept only the regular
Associationscollection, so every cross-moduleassociation pointing at the deleted entity survived. Dropping the local FROM
end left a 16-byte pointer to an element that no longer exists, and mxbuild
11.14.0 then failed in the storage layer, above the validation that produces CE
codes — no CE number, no document named:
A password field round-tripped into a plaintext text box.
describe page→execover a Studio Pro page dropped six things withmx checkat 0 errors onboth sides, the one that matters being
IsPasswordBox True → False. CLAUDE.mdmakes describe → rename → exec the copy operation, so copying a login or
change-password page lost the masking silently.
Page round-trip fidelity
describe page→
execover a Studio Pro page reportedReplaced, notUnchanged, soADR-0008's elision could not fire and the unit churned in version control on
every re-run. Fourteen differences in four independent classes, all "the
rebuild writes a constant where Studio Pro stores a value" —
pageToGenhardcoded Autofocus, CanvasWidth and CanvasHeight, and CanvasWidth alone takes
seven distinct values across the 67 pages measured.
Page.Autofocus(11.1.0),Page.VariablesandSnippet.Variables(10.17.0) were written into olderprojects. Floors taken from the Model SDK's own
StructureVersionInfo(mendixmodelsdk 4.115.0), not from release notes — follow-up to CREATE PAGE with Params: is refused on Mendix 10 — no way to create a parameterised page #1121, which
fixed this in a page's parameters and left the header out of scope.
mxcli checkagreeing withexecMDL-PAGEARG01 refused
datagrid dg (DataSource: DATABASE …, onClick: SHOW_PAGE M.Edit(Req: $currentObject))with "not inside a data view, list view or gridrow" — on a
listview, contradicting its own wording. A list widget's onClickfires per row, so the row it renders is the context object.
execrefuses ascript whose check reports an error, so this was a blocker, not a warning: the
reporting project could not apply its slice at all.
SET DataSource = DATABASEis refused instead of wiping the source. Itpassed
check, printedAltered page …at exit 0, and left the data view withno usable datasource; the only other signal was CE7007 from mxbuild, naming the
widget and never the statement. One mapping stood in for several — a DATABASE
source has no single stored shape, the holding widget decides which element
Studio Pro writes.
Security
carrying
AutoOwnerorAutoChangedBymade mxbuild report the whole module asCE0066 "Entity access is out of date", and
UPDATE SECURITY— the documentedrepair for exactly that error — printed "Reconciled 1 access rule(s)" and left
it standing, because it re-added the entry that caused it. Four lines on a
clean production-security app reproduce it.
Lint
staticimage/dynamicimage),which the React client does not support. In the linter rather than
checkdeliberately: describe → exec of a legacy page is a legitimate lossless
operation that a check warning would flag every time. Marketplace modules are
excluded, so it stays off Studio Pro content the reader cannot fix. Also
corrects a version claim carried in three places — the React client was added
in 10.7, so CE0582 applies from there, not "in Mendix 11".
One net-zero pair, kept for honesty
3ac6df51and its revertc2755dfe. The hypothesis — that CodeQL read aninline
w["IsPasswordBox"]map index as a credential lookup — was wrong: there-run produced a byte-identical alert, and running the query locally showed the
source is the generated property descriptor in
modelsdk/gen. The alertreproduces on unmodified
main, so it was not that branch's to fix. Net diff isnothing.