Repository navigation
fix: refuse a page, snippet or layout rewrite that would delete widgets MDL cannot author - #1566
Merged
Merged
Conversation
…ts MDL cannot author `describe page M.P` prints a classic data grid, a reference selector, a drop-down or a reference set selector as a `-- NOT re-executable` comment, and executing that output replaced the page with the widget gone — only "Replaced page M.P", mx check clean on both sides (mendixlabs#1622, #1554). A `create or modify` / `create or replace` of a stored page, snippet or layout that holds such a widget, and does not declare it by name, is now refused (guard-don't-drop, ADR-0005). The refusal lists every widget, the `alter` that edits the document while keeping them, and the `alter … { drop … }` that removes them on purpose first. `check -p` reports the same, following an earlier `alter` of the document in the script. The widgets come from describe itself: describeDroppedWidget already runs for exactly the widgets MDL cannot author, and now also feeds the guard, which describes the stored document into io.Discard. Layout describe goes through describeWidgetBody so the guard sees its widgets too. The #1415 round-trip test asserted the old behaviour (the description of a page with a legacy grid executes); it now asserts the refusal, that the #1415 dangling-selection error stays gone, and that the grid is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUbjJ5DeS2vysn9hfKir4Y
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.
Closes #1554 (mendixlabs#1622)
What it does
describe page M.Pprints a classic data grid, a reference selector, a drop-down or a reference set selector as a-- NOT re-executablecomment. Executing that output replaced the page with the widget gone. The only output wasReplaced page M.P, with no warning, andmx checkwas clean on both sides.The issue offered two remedies: refuse, or keep the widgets. This PR refuses, which is the repo's guard-don't-drop posture (ADR-0005, the same as
validate_workflow_rewrite.go). Acreate or modify/create or replaceof a stored page, snippet or layout is refused when the stored document holds such a widget and the statement doesn't declare it by name.The refusal says what to do next:
check -preports the same before anything is written. This is the "pre-check" the issue asks for. It runs in statement order and skips a document that an earlieralter,createordropin the same script touches.describeDroppedWidgetalready ran for exactly these widgets; now it also feeds the guard, which describes the stored document intoio.Discard. There is no second list of authorable$Types to drift from the describe switch.describeWidgetBody; its output is identical when nothing is dropped.Keeping the widgets in place across a rewrite would need new MDL syntax for "the stored widget here" and a raw-BSON splice that carries selection references. That is a larger proposal, not this fix.
Testing
make build,make test,make lint(including conformance),make check-findingsandmake test-integration-roundtripall pass locally.mdl/executor/issue1622_page_rewrite_unauthorable_test.go. They run on a real backend against a fixture copy, with a Studio Pro-shapedForms$DataGrid(the [up#1565] A fall-through error handler on an activity inside an IF branch is wired to the wrong place: an endless loop with a result variable, a silent end without one #1459 shape) andForms$ReferenceSelectorspliced into the stored unit. No fixture project holds these widgets.check -preports it.alter … dropthen rewrite goes through.alterin the same script is honoured by check.Replaced page MyFirstModule.Orders,the stored snippet lost its data grid,the stored layout lost its reference selector. The two "goes through" tests pass both ways; they are the controls against over-refusal.mdl/roundtrip/dangling_selection_1415_test.goasserted the old behaviour, namely that the description of a page with a legacy grid executes, which is this bug. It now asserts three things: the refusal names the grid, the [up#1533] Two unhelpful parse messages in workflow MDL (reserved activity name close, exposed before returns) #1415 dangling-selection error stays gone, and the stored page keeps its grid.make test-integration-executorhad one local failure,TestMxCheck_DoctypeScripts(REST/workflow property sets), which this change doesn't touch. I ran it on a cleanorigin/mainworktree with the same mxbuild 11.6.6 and it fails the same way. CI pins 11.12.2.Mendix validation
This is a refusal on the mxcli side, so nothing new is written. The repro in the issue (Mendix 10.24, MPR v1) isn't available here. No fixture holds these widgets, so they were seeded into stored units.
Agentic Code Testing
alter-page: a page holding such widgets refuses the rewrite; usealter)create-page.mdandalter-layout.md, CHANGELOG, bug-testmdl-examples/bug-tests/1622-rewrite-keeps-unauthorable-widgets.mdlGenerated by Claude Code