Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/skills/fix-issue/findings/mdl-executor.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -709,4 +709,5 @@
{"area": "mdl/executor", "symptom": "`alter styling … set 'Spacing bottom' = 'Outer medium'` (an Atlas Core 4.1.3 old name) warned MDL-WIDGET11 \"which no widget type in this project's theme declares — mxbuild reports this as CE6083\"; mxbuild actually reports CE6087 \"Design properties have been renamed in your theme\"", "cause": "validateAlterStylingDesignProps checked only current property names across all widget groups; #679 taught the authoring paths the theme's oldNames but not ALTER STYLING, which then offered a spelling near-miss instead of the current property", "file": "`mdl/executor/validate_alter_styling.go` (`renamedAnywhere`, `renamedStylingSuggestion`)", "insight": "**Pick the real-run example by what the resolver can see**: ALTER STYLING knows only a widget NAME, so it asks the whole theme — and Atlas 4.1.3 declares a CURRENT 'Align content' on the Image widget while 'Align content' is an old name on DivContainer, so that example is (correctly) silent under the under-report policy; 'Spacing bottom' is declared nowhere under a current name and exercises the rename path. A renamed key whose replacement is a compound (Spacing side, multi-select option) cannot be written by ALTER STYLING's one flat value, so the suggestion must point at the inline DesignProperties form rather than a `set` it cannot execute. Real run: old name via exec → CE6087; the suggested inline form → 0 errors", "refs": ["ako/mxcli#679"], "rules": ["MDL-WIDGET11"], "date": "2026-09-25"}
{"area": "mdl/executor", "date": "2026-09-25", "symptom": "A `label` widget's DesignProperties were never checked: describe → check --references of FeedbackModule.ShareFeedback_Logo (Feedback v4.0.2, Atlas Core 4.1.3, Mendix 11.13.0) warned MDL-WIDGET11 'renamed' on the containers but not on label1's 'Spacing bottom': 'Outer none' (CE6087 in mxbuild). Same gap on the write side: `label l (DesignProperties: ['Style': '#ff0000'])` checked clean, exec'd, and failed mx check with [CE6085] \"Unknown option #ff0000 for design property Style.\" at Label", "cause": "The `label` keyword (PR #670, writes Forms$Label) was never added to mdlKeywordToDesignPropsKey, so resolveDesignPropsKey returned 'label' — no design-properties.json key. validateWidgetDesignProps skips a widget whose key is absent (meant for unknown pluggables), and the builder's GetPropertiesForWidget returned only the 'Widget' base group, so the Label's own ColorPicker 'Style' was unknown and a free colour fell to the Option default. bsonTypeToDesignPropsKey already had Forms$Label → Label; only the keyword half was missing", "file": "mdl/executor/theme_reader.go", "insight": "Adding a native widget keyword has a third registration nobody asks for: mdlKeywordToDesignPropsKey. Missing it is silent in both directions — the validator's 'no theme key → skip' rule turns an unmapped keyword into approval, and the builder still gets the Widget base group, so Spacing/Hide on write correctly and only the type-specific properties (Label's ColorPicker Style) mis-type. Test a type-specific property with an off-list value; a Widget-base property passes either way. Quick audit: a keyword should resolve to the same key as its stored $Type — groupbox (GroupBox, which has a ColorPicker Style), tabcontainer, navigationtree, menubar, simplemenubar, row/column (LayoutGridRow/Column) are still unmapped. Also seen: MDL-WIDGET12 warns on a ColorPicker free colour that the builder writes as Custom and mxbuild accepts (0 errors) — a pre-existing false positive, not changed here", "refs": ["#670", "#679"], "rules": ["MDL-WIDGET11", "MDL-WIDGET12"], "ce": ["CE6085", "CE6087"]}
{"area": "mdl/executor", "date": "2026-09-25", "symptom": "`mxcli check` passed a microflow with a commit inside a loop \u2014 one database round trip per iteration \u2014 that `mxcli lint` already flagged as CONV011. The defect surfaced only at project-wide lint time, long after the write.", "cause": "CONV011 reads the STORED model, so it cannot speak until `exec` has written the microflow; `check` reads the MDL and had no equivalent rule. The gap is temporal, not a missing capability on either side.", "file": "`mdl/executor/validate_commit_in_loop.go` (MDL-PERF01, hooked in `validate_microflow.go`), test `validate_commit_in_loop_test.go`, example `mdl-examples/bug-tests/1186-commit-in-loop.mdl`", "insight": "**When adding a check-time rule that anticipates an existing lint rule, pin the BOUNDARY to the lint rule's, not to the better one, and say why in the code.** A `while true` is built as an ExclusiveMerge back-edge rather than a LoopedActivity, so CONV011 (which walks LoopedActivity) does not flag a commit inside one. A commit there is arguably still N+1, and the tempting move is to be more correct \u2014 but two rules for one concept that disagree on what counts is precisely how a pair drifts, and this repo already has CONV010's three successive short allowlists as the worked example. If the case is worth reporting it is worth reporting in BOTH, and the stored-model rule is the one that sees the built flow. The test that pins this carries a control on the control: `while true` is exempt, `while <real condition>` is not, so the exemption cannot silently become 'never flag a while'. Name the sibling rule in the message (`lint reports this as CONV011`) so a reader hitting one recognises the other rather than filing it twice. Also worth reusing: `checkReturnInLoop` already had the depth-tracking walk over Loop/While/If/EnumSplit/InheritanceSplit \u2014 copying its shape got the nesting cases right for free.", "refs": ["ako/mxcli#681", "mendixlabs/mxcli#1186"], "rules": ["MDL-PERF01"]}
{"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]}
{"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []}
76 changes: 76 additions & 0 deletions mdl-examples/bug-tests/alter-page-unscoped-insert-bindings.mdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
-- ============================================================================
-- ALTER PAGE INSERT/REPLACE where no entity is in scope: bindings qualified
-- ============================================================================
--
-- Symptom: on FeedbackModule.ShareFeedback_Logo (Feedback v4.0.2, Mendix
-- 11.13.0), whose data view is sourced by a nanoflow the module does not ship,
-- alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 {
-- image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }
-- passed `mxcli check --references`, and exec wrote the parameter as a BARE
-- attribute reference: `mx check` could no longer LOAD the project
-- (ArgumentNullException setting 'Attribute'). The same happens to a widget
-- inserted outside every data container; a text box's `Attribute:` there is
-- dropped instead (CE7005 "No value selection has been made").
--
-- Cause: ALTER takes its entity from the STORED document (the enclosing data
-- source, or a flow source's return type). With none, the builder has nothing
-- to qualify a bare name against. CREATE PAGE already refused this at check
-- time; ALTER's check never opened the document.
--
-- Fix: `check --references` opens the stored page, resolves the insertion
-- point's entity through the same function exec uses (alterEntityContext), and
-- refuses bare bindings when there is none, naming widget and binding.
--
-- Verify: exec with -p → both ALTERs apply, `mxcli docker check` → 0 errors.
-- Change the qualified binding in the first ALTER to `{1} = Subject`, or
-- insert `textbox t (Attribute: Subject)` after ctTop (no data container) →
-- `check --references` reports it. The bare name in the second ALTER (inside
-- dvEntity, entity in scope) stays unflagged.
-- ============================================================================

create module BugTestAlterScope;
/

create persistent entity BugTestAlterScope.Draft (
Subject: String(200)
);
/

@excluded
create or modify page BugTestAlterScope.Draft_Example
( Title: 'Draft (example)', Layout: Atlas_Core.Atlas_Default,
Params: { $Draft: BugTestAlterScope.Draft } )
{
container ctTop {
dynamictext txtIntro (Content: 'Example')
}
layoutgrid lg {
row r {
column c (DesktopWidth: AutoFill) {
dataview dvFlow (DataSource: nanoflow BugTestAlterScope.DS_MissingForm) {
textbox txtSubject (Label: 'Subject', Attribute: BugTestAlterScope.Draft.Subject)
}
dataview dvEntity (DataSource: $Draft) {
textbox txtSubject2 (Label: 'Subject', Attribute: Subject)
}
}
}
}
}
/

-- Inside the data view whose flow is missing: the binding must be qualified.
alter page BugTestAlterScope.Draft_Example {
insert after txtSubject {
dynamictext txtEcho (Content: 'About: {1}', ContentParams: [{1} = BugTestAlterScope.Draft.Subject])
}
};
/

-- Inside a data view whose entity is known: a bare name is fine.
alter page BugTestAlterScope.Draft_Example {
insert after txtSubject2 {
dynamictext txtEcho2 (Content: 'About: {1}', ContentParams: [{1} = Subject])
}
};
/
52 changes: 31 additions & 21 deletions mdl/executor/cmd_alter_page.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,19 +349,7 @@ func applyInsertWidgetMutator(ctx *ExecContext, mutator backend.PageMutator, op
// target IS the container, so the children take the target's own context (e.g.
// a dataview's entity).
into := strings.EqualFold(op.Position, "INTO")
entityCtx := mutator.EnclosingEntity(op.Target.Widget)
if into {
entityCtx = mutator.EnclosingEntityForChildren(op.Target.Widget)
}
// A microflow/nanoflow datasource contributes no entity to the BSON walk (its
// entity is the flow's RETURN type), so resolve it via the model — otherwise a
// widget inserted into a flow-sourced list binds nothing (CE0402/CE1613). (#55)
if entityCtx == "" {
mfQN, nfQN := mutator.EnclosingDataSourceFlow(op.Target.Widget, into)
if e := resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mfQN, nfQN); e != "" {
entityCtx = e
}
}
entityCtx, _ := alterEntityContext(ctx, mutator, op.Target.Widget, into, moduleName, moduleID)

// Build new widgets from AST
widgets, err := buildWidgetsFromAST(ctx, op.Widgets, moduleName, moduleID, entityCtx, mutator)
Expand Down Expand Up @@ -435,14 +423,7 @@ func applyReplaceWidgetMutator(ctx *ExecContext, mutator backend.PageMutator, op
}

// Find entity context from enclosing DataView/DataGrid/ListView for regular widget replace.
entityCtx := mutator.EnclosingEntity(op.Target.Widget)
// Resolve a microflow/nanoflow datasource's return entity (see the INSERT path).
if entityCtx == "" {
mfQN, nfQN := mutator.EnclosingDataSourceFlow(op.Target.Widget, false)
if e := resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mfQN, nfQN); e != "" {
entityCtx = e
}
}
entityCtx, _ := alterEntityContext(ctx, mutator, op.Target.Widget, false, moduleName, moduleID)

// Build new widgets from AST, excluding the target widget/column from the
// duplicate-name scope so a same-name replacement is allowed.
Expand Down Expand Up @@ -578,6 +559,35 @@ func buildColumnSpecsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleNa
// Widget building from AST (domain logic stays in executor)
// ============================================================================

// alterEntityContext is the entity an INSERT or REPLACE builds its widgets
// against, plus the flow that was meant to supply it when a flow data source is
// where the scope comes from. forChildren is INSERT INTO: the target IS the
// container, so its own data source decides; otherwise (INSERT BEFORE/AFTER,
// REPLACE) the target is a sibling and the nearest ENCLOSING source does.
//
// A microflow/nanoflow datasource contributes no entity to the BSON walk (its
// entity is the flow's RETURN type), so it is resolved via the model — otherwise
// a widget inserted into a flow-sourced list binds nothing (CE0402/CE1613, #55).
//
// Shared with the check-time pass (validate_alter_unscoped.go), so check and exec
// cannot disagree about which entity is in scope.
func alterEntityContext(ctx *ExecContext, mutator backend.PageMutator, widgetRef string, forChildren bool, moduleName string, moduleID model.ID) (entity, flow string) {
if forChildren {
entity = mutator.EnclosingEntityForChildren(widgetRef)
} else {
entity = mutator.EnclosingEntity(widgetRef)
}
if entity != "" {
return entity, ""
}
mfQN, nfQN := mutator.EnclosingDataSourceFlow(widgetRef, forChildren)
flow = mfQN
if flow == "" {
flow = nfQN
}
return resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mfQN, nfQN), flow
}

// resolveDataSourceFlowEntity resolves the entity context contributed by a
// microflow/nanoflow datasource — its RETURN entity — for ALTER PAGE widget
// builds. A flow datasource stores no entity in its own BSON (the entity lives
Expand Down
44 changes: 28 additions & 16 deletions mdl/executor/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,11 @@ func validateProgramWithWarnings(ctx *ExecContext, prog *ast.Program) ([]error,
// widget that is already stored, so its property can only be resolved
// against the document — which is why it passed check and failed exec.
errors = append(errors, validateAlterSetProperties(ctx, prog, sc)...)
// The entity an ALTER's INSERT / REPLACE binds against is in the stored
// document, not the statement. Where it has none (a missing flow source, or
// 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)...)
return errors, sc.warnings
}

Expand Down Expand Up @@ -940,21 +945,6 @@ func (sc *scriptContext) relaxExcludedWidgetRefs(kind, name string, widgets []*a
// the page writer's refusal of a bare attribute reference.
func unscopedBindings(widgets []*ast.WidgetV3, ref string) []string {
var out []string
var inScope func(ws []*ast.WidgetV3)
inScope = func(ws []*ast.WidgetV3) {
for _, w := range ws {
if w == nil {
continue
}
if _, own := w.Properties["DataSource"].(*ast.DataSourceV3); own {
continue // its own data source decides its children's scope
}
for _, b := range bareBindingsOf(w) {
out = append(out, fmt.Sprintf("%s `%s` (%s)", strings.ToLower(w.Type), w.Name, b))
}
inScope(w.Children)
}
}
var find func(ws []*ast.WidgetV3)
find = func(ws []*ast.WidgetV3) {
for _, w := range ws {
Expand All @@ -966,7 +956,7 @@ func unscopedBindings(widgets []*ast.WidgetV3, ref string) []string {
for _, b := range bareBindingsOf(w) { // the container's own bindings, e.g. its visibility
out = append(out, fmt.Sprintf("%s `%s` (%s)", strings.ToLower(w.Type), w.Name, b))
}
inScope(w.Children)
out = append(out, bindingsWithoutScope(w.Children)...)
continue
}
find(w.Children)
Expand All @@ -976,6 +966,28 @@ func unscopedBindings(widgets []*ast.WidgetV3, ref string) []string {
return out
}

// bindingsWithoutScope names the bare bindings of widgets placed where no
// entity is in scope — the children of a container whose data source flow is
// missing, or widgets an ALTER inserts outside any container that resolves to
// an entity. Descent stops at a widget with a data source of its own, which
// scopes its children.
func bindingsWithoutScope(ws []*ast.WidgetV3) []string {
var out []string
for _, w := range ws {
if w == nil {
continue
}
if _, own := w.Properties["DataSource"].(*ast.DataSourceV3); own {
continue // its own data source decides its children's scope
}
for _, b := range bareBindingsOf(w) {
out = append(out, fmt.Sprintf("%s `%s` (%s)", strings.ToLower(w.Type), w.Name, b))
}
out = append(out, bindingsWithoutScope(w.Children)...)
}
return out
}

// bareBindingsOf lists a widget's attribute bindings that need an entity in
// scope to resolve.
func bareBindingsOf(w *ast.WidgetV3) []string {
Expand Down
Loading
Loading