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
2 changes: 2 additions & 0 deletions .claude/commands/mxcli-dev/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ proactively. Add a row after every review that surfaces something new.
| 29 | A predicate that names ONE cause of a build error is read as if it named the error (`mem.IsCalculated` for CE6592, which an autonumber also triggers) — the half that is covered works, so every test passes and the gap is invisible until a user hits the other half | Code correctness | When a guard cites a CE number, enumerate what the PLATFORM rejects, not what the current code checks. Put the rule in one named place (`types.WriteRightsForbidden`) rather than a bare boolean at each site, so the second cause has somewhere to go. And fix every pass that can re-derive the value — a reconcile running after every program re-broke a grant the user had corrected by hand |
| 30 | Two commands compute the same thing from two copies of the setup (`report` re-implementing `lint`'s rule list and skipping its config), so they disagree about a project — and a SCORE carries no provenance, so neither number looks wrong | Code correctness | Extract the shared setup and route both through it. A value test cannot guard this when the copies live inside cobra `RunE` bodies: use a structural check on the source, with a positive control asserted FIRST so it cannot pass vacuously |
| 31 | A test helper that needs a heavyweight object only to satisfy a signature (`NewLintContext(nil, nil)`, which panics) invites a nil-guard added purely to make the test compile — behaviour nothing in production needs, defended forever | Test coverage | Narrow the signature instead: if the helper does not use the parameter, drop it and let the caller apply the part it owns. A test that cannot construct an argument is usually telling you the argument does not belong |
| 32 | A fix adds a diagnostic for a capability the model lacks while leaving in place the code that asserts the capability EXISTS — MDL042 telling the author a loop's `@caption` is dropped, while `cmd_microflows_builder_annotations.go` still ran `case *microflows.LoopedActivity: activity.Caption = ann.Caption` under the comment "LOOP / WHILE activities can carry a caption just like splits", and the describer still emitted one. Nothing read either back. The next reader trusts the code over the warning, deletes the check, and reopens the bug from the other side. The reason it survives is that it usually has TESTS — three here asserted the caption was carried, all of them against the semantic object and none against storage, so they passed throughout and failed only on the correct fix | Code correctness | `generated/metamodel` is the arbiter: a field on the semantic type it does not declare cannot survive a write, so an assignment to it is dead by construction. Grep the writer, the describer and the semantic struct and delete (or re-comment) whatever sets it. MEASURE before deleting — `exec` then `describe` on a real project, with the UNMODIFIED build, so the deletion rests on the stored document rather than on reading the codec. Invert the tests that defended it rather than deleting them, keeping any half still true (escaping coverage belongs on a type that can carry a caption), and check the inverted test fails when the assignment is put back |
| 33 | A column added to `createTables` without bumping `CatalogSchemaVersion` — the version guard only drops tables when it CHANGES, and `CREATE TABLE IF NOT EXISTS` never adds a column, so every user with a cached catalog keeps a table the new SELECT cannot read. Measured on #1181: `activities_for` yielded 302 on `main` and **0** on the branch against the same cache, `no such column: UseRequestTimeout`, `mxcli lint` exit 1. `mxcli report` runs the same rules and never checks `QueryErrors()`, so there it would score silently | Code correctness | Bump the constant in the same commit — its doc comment says so and `62913741` is the precedent (four columns + 11→12 + a builder test). To REPRODUCE, the cache must actually be reused: build it with the old binary and run the new one with the **same spelling of `-p`**, because a relative-vs-absolute path invalidates on "MPR path changed" and hides the bug; confirm the run says "Loading cached catalog … (from cache)" before believing a green result |

---

Expand Down
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 @@ -689,6 +689,7 @@
{"area": "mdl/executor", "date": "2026-09-23", "symptom": "`CALL MICROFLOW M.F(…) IN QUEUE M.Q` where `F` returns **Boolean**: `mxcli check -p --references` says `Check passed!`, `mx check` says **CE7033** \"A microflow used for background execution must have a Microflow return type of 'Nothing'.\" (at Call microflow activity 'F'). Reported with the CE0142 after-startup sibling, which MDL073 had already closed.", "cause": "The --references pass resolved the call target and the queue name separately, and both resolve. Nothing compared the binding (`in queue`) against the signature of the flow it names. Added MDL088: a project-less pass (ValidateQueuedCallReturnType) for a target the script creates, and validateQueuedMicroflowTargets on the --references path for a stored target, which skips script-defined targets so the fault is not printed twice. Stored void microflows read back as ReturnType \"Void\", not \"\" — both must mean Nothing.", "file": "`mdl/executor/validate_queued_call_return.go` (queuedMicroflowCalls, checkQueuedMicroflowReturnsNothing, validateQueuedMicroflowTargets, ValidateQueuedCallReturnType), wired in `validate_program.go` and `validate.go` (validateFlowBodyReferences); examples `mdl-examples/bug-tests/1064-queued-microflow-must-return-nothing{,.fail}.mdl`", "insight": "Same class as MDL073 (\"the reference resolves\" ≠ \"the reference is usable\"): any binding that names a flow carries a constraint on that flow's signature, and a resolver checks only the name. When one such check lands, sweep for its siblings at other binding sites. The queued CALL JAVA ACTION twin (CE7038) is still unchecked and was deliberately left out of scope. Two things that cost time: (1) `mxcli exec` of a script that CREATEs a queue and then binds a call to it refuses with 'task queue not found' — validateFlowBodyReferences checks queues against the project only, not the script context — so the repro has to create the queue in a separate exec; (2) walk call statements by reflection, not by the flowRefCollector switch, which does not descend into WHILE bodies. Measured on mxbuild 11.12.0 with two projects: Boolean target → CE7033, void target → 0 errors.", "refs": ["mendixlabs/mxcli#1064"], "ce": ["CE7033"], "rules": ["MDL088"]}
{"area": "mdl/executor/microflow-layout", "date": "2026-09-23", "symptom": "MPR011 fires on EVERY `while` loop mxcli writes \u2014 'first activity at (50,80) lies outside the loop box' \u2014 single-level loops included. `mx check` passes and the app runs; the flow just renders wrong in Studio Pro. Reported from a real project as 'looks like an mxcli layout issue', with 3 MPR011 warnings still in its final lint run. mxcli's own lint rule was correctly flagging mxcli's own output.", "cause": "One missing term in the WHILE builder. addWhileStatement had `innerStartX := LoopPadding` (50) where addLoopStatement has `LoopPadding + iteratorSpace + ActivityWidth/2` (210). A microflow object's Position is its CENTRE \u2014 the builder says so itself ('Position is the CENTER point (RelativeMiddlePoint in Mendix)') \u2014 so a centre at x=50 with ActivityWidth=120 puts the left edge at -10. The doc comment says the while layout 'matches addLoopStatement but without iterator icon space': dropping the iterator space (100) was right, taking ActivityWidth/2 with it was not, because that term is not iterator space, it is what converts a centre to a left edge. The very next line, `innerStartY := LoopPadding + ActivityHeight/2`, adds the half-height for exactly this reason \u2014 so the omission was accidental, not a choice. Reported (50,80) matches term for term: 50 = LoopPadding, 80 = LoopPadding + ActivityHeight/2.", "file": "`mdl/executor/cmd_microflows_builder_control.go` (addWhileStatement: `innerStartX := LoopPadding + ActivityWidth/2`), tests `mdl/executor/loop_containment_test.go` (TestWhileLoopBox_ContainsDefaultLaidOutChildren, TestWhileLoopFirstChildLeftEdgeIsInsideTheBox)", "insight": "The containment invariant WAS already tested \u2014 loop_containment_test.go exists from #884 and asserts exactly this \u2014 but every fixture in it built a FOREACH loop. There are two loop builders; one was covered and the uncovered one shipped the violation into every project that writes a `while`. An invariant is worth what its COVERAGE is, and a file named for an invariant reads as if it covers the invariant, which is how a second code path goes unexamined for months. When a rule flags the tool's own output, believe the rule first: the reporter hedged with 'looks like an mxcli layout issue' and was exactly right. Cheap tell for this class: a term present on one axis and absent on the other in adjacent lines (`+ ActivityHeight/2` on Y, nothing on X) is almost always an omission rather than a decision. Failing test written first; it reproduced the reported geometry to the pixel, x[-10,...] at 1, 2, 4 and 7 activities. Still uncovered: addManualWhileTrueStatement, the third loop builder.", "refs": ["ako/mxcli#884", "ako/mxcli#645"]}
{"date": "2026-09-23", "area": "mdl-executor", "symptom": "upstream #1176: DESCRIBE prints `all` on an import activity that returns ONE object — `$objectResponse = import from mapping M.IMM($s) all;` — which reads as a list import. Reported on v0.23.0 / Studio Pro 11.12.3, after #881 was believed to have settled import ranges", "cause": "#881 made `formatImportMappingRange` always emit a range keyword, because at the time a missing keyword let the range fall back to the variable's cardinality and store First. The later runtime fix (unauthored range written as All explicitly) made bare and `all` build the same activity, but the describe side was never revisited, so `all` kept printing where it was only noise", "file": "`mdl/executor/cmd_microflows_format_action.go` (`formatImportMappingRange`: return \"\" for All against SingleObject); tests `mdl/executor/cmd_microflows_import_range_test.go` (`TestImportRange_ObjectResultDescribesWithoutAll`); example `mdl-examples/bug-tests/1176-import-mapping-object-describes-without-all.mdl`", "insight": "**This was not #881 regressing — it was #881's own workaround outliving its reason.** 'DESCRIBE must never emit nothing' was a guard against the builder's then-broken default; once the builder wrote a missing keyword as All explicitly, the guard became pure noise, and nothing linked the two sites. When a formatter emits something 'because the builder would otherwise infer X', put that reason in a test that asserts the builder equivalence (bare vs keyword build the same activity), so fixing the builder flags the formatter. Proving the omission safe needs that equivalence on a real project, not just the unit test: on 11.12.3 both spellings store byte-identical ResultHandling (ConstantRange{SingleObject:false} + ObjectType), `mx check` 0 errors, and exec'ing the described text reports 'Unchanged microflow'. Wrong turn to skip: a JSON diff of two EMPTY extractions prints 'IDENTICAL' — `bson dump` emits ordered Key/Value lists, not objects; check the extraction is non-empty before trusting a diff", "refs": ["#881", "#1176"]}
{"area": "mdl/executor", "date": "2026-09-24", "symptom": "`@caption 'Are there months left?'` above a `while` passed `mxcli check` with no warning, `exec` created the microflow, and `describe` showed the while with no caption. The same caption on a `loop` was reported as MDL042.", "cause": "MDL042 lived in the `*ast.LoopStmt` case of validate_microflow.go only. `addWhileStatement` builds the same Microflows$LoopedActivity as a for-each loop -- a WhileLoopCondition instead of an iterator -- and LoopedActivity has no Caption property, so the caption had nowhere to go and nothing said so.", "file": "mdl/executor/validate_microflow.go", "insight": "A diagnostic keyed on one AST statement misses every other statement that builds the same model element. When a check exists because the MODEL lacks a property, key it on what the builder writes (here: every LoopedActivity) rather than on the MDL keyword that led there.", "refs": "mendixlabs/mxcli#1187"}
{"area": "mdl-executor", "date": "2026-09-24", "refs": ["#1173"], "symptom": "`ALTER ENTITY <view> ADD ATTRIBUTE Region: String(200)` passed `mxcli check -p --references`, exec printed \"Added attribute 'Region' to entity MyFirstModule.SaleStats\" with exit 0, and `mx check` then failed CE6770 \"View Entity is out of sync with the OQL Query.\" The attribute was written as DomainModels$StoredValue with no OQL column behind it", "cause": "execAlterEntity's ADD/DROP ATTRIBUTE branches treat every entity as a table: nothing asked isViewEntity, although CREATE ASSOCIATION and bulk ALTER ENTITIES already did. The check-time AlterEntityStmt case only resolved the module and enumerations", "file": "`mdl/executor/cmd_entities.go` (viewEntityAttributeSetRefusal, AlterEntityAddAttribute/DropAttribute guards), `mdl/executor/validate.go` (validateViewEntityAttributeSet); test `mdl/executor/alter_entity_view_test.go`; bug-test `mdl-examples/bug-tests/1173-alter-view-entity-attribute.mdl`", "insight": "**Measure every ALTER op on a view before choosing the fix, not just the reported one** — one mxbuild per op on 11.12.1: ADD → CE6770, DROP → CE6770, MODIFY to the wrong type → CE6770 but MODIFY to the matching type → 0 errors (so MODIFY's failure is a type mismatch, a different gap, not this one), RENAME → 0 errors (the OqlViewValue binds the column by its Reference/alias, not the attribute name), SET COMMENT → 0. Refusing all four attribute ALTERs would have blocked a working RENAME. **Refuse rather than bind**: the issue offers \"create an OqlViewValue bound to the matching alias\", but for ADD there is no matching alias — the query has no such column — so any write stays CE6770; the query is the declaration, so the refusal points at `create or modify view entity`, verified to build clean with the added column. Guard both layers: check must see a view the SCRIPT creates (sc.viewEntities) as well as a stored one (findEntity + isViewEntity), or check passes a script exec stops halfway. Bulk ALTER ENTITIES already excluded views (e.Source/OqlQuery), so the single-entity path was the only entry. Control: stubbing both guards fails all four refusal tests with \"accepted — mxbuild reports CE6770\""}
{"date": "2026-09-24", "area": "mdl-executor", "symptom": "upstream #1175: a `--` comment inside a view entity's select list produced false MDL030 — `select column 1 has no as alias: '-- the customer's running total'` plus a second one for the text after the comment's comma. Reported on v0.23.0 as an apostrophe bug", "cause": "Every static OQL check (ValidateOQLSyntax, ValidateOQLTypes, inferOQLTypes, viewAssociationColumns) works on `Query.RawQuery`, which is stored verbatim and so keeps the author's comments. parseSelectColumns splits on top-level commas with no notion of a comment, so the comment became a column and each comma in it another", "file": "`mdl/executor/oql_comments.go` (`stripOQLComments`, called at the top of the four entry points in `oql_type_inference.go` / `oql_view_associations.go`); tests `mdl/executor/validate_oql_comments_test.go`; example `mdl-examples/bug-tests/1175-oql-comment-is-not-a-select-column.mdl`", "insight": "**The apostrophe was a red herring: a comment with no apostrophe fails the same way** — measured before theorising, and it moved the fix from the quote-skipping in topLevelKeywordIndex to the comment itself. Strip at the entry points, not inside the helpers: the checks also run regexes over the whole query (division, association-path, reserved word), and a comment containing `from`, `/` or `a.B.C_D` would trip those too. Blank comments to spaces of the same length rather than deleting them, so any offset computed on the stripped text still indexes the original. Do NOT strip in the visitor — the stored query keeps the comments, which is the author's documentation. A type-check test using the same query passed without the fix (comment columns infer no type), so it was dropped rather than kept as a test that detects nothing", "refs": ["mendixlabs/mxcli#1175"], "rules": ["MDL030"]}
{"area":"mdl/executor","date":"2026-09-24","symptom":"`mxcli check -p … --references` passes a view entity whose association-path join is written `join s/Mod.A_B/System.UserRole AS r` (uppercase AS) even when a pass-through column from `r` declares the wrong string length; mxbuild then fails with CE6770 \"View Entity is out of sync with the OQL Query.\" Lowercase `as` reports MDL031 correctly.","cause":"extractAliasMap matched the path join case-insensitively ((?i)…(?:as\\s+)?) but then recovered the path from match[0] with strings.TrimSuffix(path, \"as\") — case-sensitive — so with `AS` the path kept a trailing ` AS`, the end-anchored lastEntity regex failed, and the alias was never mapped. Every column from that alias silently went without type inference.","file":"mdl/executor/oql_type_inference.go","insight":"A (?i) regex followed by string surgery on the whole match reintroduces case sensitivity by the back door: capture every piece you need as its own group instead of trimming it back out. The tell is a single control table varying only the case of one keyword — every other keyword in upper case was harmless, which points straight at code that handles that one token outside the regex. An unresolved alias is silent (the checker skips unknown types rather than reporting), so the symptom is a check that PASSES; test at extractAliasMap directly, and control with the unfixed binary on a real project (it printed `Check passed!` for AS, the error for as). DESCRIBE prints AS in upper case, so round-tripped OQL hits this by default.","refs":["#652"],"ce":["CE6770"],"rules":["MDL031"]}
Expand Down
3 changes: 3 additions & 0 deletions .claude/skills/mendix/write-lint-rules/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@ def count_not(node):
| `entity_ref` | string | Referenced entity qualified name |
| `service_ref` | string | Called service document (REST / web service / OData client); empty when the activity calls none |
| `action_ref` | string | Operation or action within that service; empty when the activity calls none |
| `use_request_timeout` | bool | Call REST service: whether "Use a timeout" is enabled. False for other action types |
| `timeout_expression` | string | Call REST service: the timeout in seconds, stored as an expression, e.g. `"300"` |

### rest_client
| Property | Type | Example |
Expand Down Expand Up @@ -395,6 +397,7 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific)
| `member_name` | string | Attribute name (for MEMBER_READ/MEMBER_WRITE) |
| `xpath_constraint` | string | XPath constraint or empty |
| `is_constrained` | bool | True if XPath constraint is set |
| `default_member_access_rights` | string | The rule's "default rights for new members": `"None"`, `"ReadOnly"` or `"ReadWrite"`. Empty for non-entity permissions |

### user_role
| Property | Type | Example |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,11 @@ begin
end loop;
```

> **`@caption` does nothing on a loop.** Mendix for-loops have no caption
> property, so `@caption` on a `loop` is silently dropped (`mxcli check` flags
> it as **MDL042**). To label a loop, use `@annotation 'text'` — it attaches a
> note, exactly like drawing one onto the loop in Studio Pro.
> **`@caption` does nothing on a loop or a while loop.** Both are the same loop
> activity, which has no caption property, so `@caption` on a `loop` or a `while`
> is dropped (`mxcli check` flags it as **MDL042**). To label either, use
> `@annotation 'text'` — it attaches a note, exactly like drawing one onto the loop
> in Studio Pro.

**Note**:
- Loop variable (`$Product`) is scoped to the loop body
Expand Down
Loading
Loading