diff --git a/.claude/commands/mxcli-dev/fix-issue.md b/.claude/commands/mxcli-dev/fix-issue.md new file mode 100644 index 0000000000..ba9bfb8cc8 --- /dev/null +++ b/.claude/commands/mxcli-dev/fix-issue.md @@ -0,0 +1,88 @@ +--- +description: Diagnose and fix a reported bug, with the evidence bar the PR checklist expects +--- + +# /mxcli-dev:fix-issue — Fix a Reported Bug + +Work a bug report end to end: match it against what has already been seen, fix it, +and prove the fix with evidence a reviewer can check. + +Read [`.claude/skills/fix-issue.md`](../../skills/fix-issue.md) first — it holds the +findings-lookup mechanics and the two rules that are easiest to skip. This command +is the order to do things in. + +`$ARGUMENTS` is the issue number or a description of the symptom. + +## Steps + +1. **Read the issue in full**, including comments. Quote the reported symptom + verbatim somewhere — it is what the test has to reproduce, and paraphrasing it + is how a fix ends up addressing a different bug. + +2. **Match the failure class**, then the instance: + - `docs-wiki/bug-patterns/` for the class (small, read it) + - `grep -il '' .claude/skills/fix-issue/findings/*.jsonl` + for the instance + + A pattern-page miss means "not yet digested", never "not seen before". + +3. **Establish the version facts before theorising**, if the report involves one. + The arbiter for a metamodel property is the Mendix Model SDK's own + `StructureVersionInfo` (`npm pack mendixmodelsdk`, then grep `src/gen/*.js` for + the type) — not release notes, and not a number already written down in this + repo. mendixlabs/mxcli#1121 was a floor copied from a proposal's illustrative + sample output that nobody had ever measured. + +4. **Write the failing test first**, at the layer the symptom lives in. Table of + layer → package is in the skill. + +5. **Prove the test detects the bug.** Revert the fix, or stub the guard, and + confirm it fails *with the reported symptom*. Put the control's output in the PR. + A test that has only ever run against fixed code has not been shown to detect + anything. + +6. **Run the real thing when the argument calls for it.** Required when the fix's + justification asserts what a Mendix tool accepts or rejects — a version guard, a + refusal, anything resting on "mxbuild would catch this". Also when the symptom is + a property of the running app (`verify-in-runtime.md`). Cheapest form: + + ```bash + mxcli new --version --theme none --layout none --skip-init + mxcli exec .mdl -p .mpr + mxcli docker check -p .mpr # and again with the fault forced back in + mxcli run --local -p .mpr # when it has to render + ``` + + Build **two** copies — fixed, and with the fault forced back in — or the run + tells you nothing you did not already believe. + +7. **Add the regression case**: `mdl-examples/bug-tests/-.mdl`, + and check it parses (`mxcli check`) and passes `make check-mdl`. + +8. **Append one finding** to `.claude/skills/fix-issue/findings/.jsonl` and run + `make check-findings`. Write the insight — what would have made this cheaper to + find, and which plausible wrong turn to skip — not the changelog. + +9. `make build && make test && make lint`, then commit. + +## Before you say it cannot be verified + +That claim ends the investigation, so it needs the evidence a fix would. State what +a positive result would look like; try the case you are sure works as a control; try +a different *shape* of query rather than another value. Then, if it still holds, say +it with the evidence attached rather than as a property of the environment. + +On #1121 the claim was "Mendix 10 is not downloadable here", from a dozen uniform +404s with 11.x succeeding on the same host. It was wrong — Mendix 9 and 10 publish +four-part names with a build number — and a uniform negative across a whole class +was the tell that the query, not the class, was at fault. + +## Done when + +- [ ] Reported symptom reproduced by a test before the fix existed +- [ ] Control run recorded (reverted fix → test fails with that symptom) +- [ ] Full run done, with both variants, if the argument asserts what a tool accepts +- [ ] Any "cannot verify" claim carries its evidence and a falsifying control +- [ ] Bug-test MDL committed; `make check-mdl` passes +- [ ] Finding appended; `make check-findings` passes +- [ ] `make build && make test && make lint` pass diff --git a/.claude/commands/mxcli-dev/proposal.md b/.claude/commands/mxcli-dev/proposal.md index 5c8df7078f..24dd12398e 100644 --- a/.claude/commands/mxcli-dev/proposal.md +++ b/.claude/commands/mxcli-dev/proposal.md @@ -25,7 +25,7 @@ Ask the user: If the user isn't sure about version or BSON, help them find out: - Version: check `reference/mendixmodellib/reflection-data/` or Mendix release notes -- BSON: check if similar features exist in `sdk/mpr/parser*.go` or `sdk/mpr/writer*.go` +- BSON: check if similar features exist in `mdl/backend/modelsdk/*_read.go` or `*_write.go` ### Phase 2: BSON Investigation (if applicable) @@ -69,7 +69,7 @@ Before writing the proposal, search for existing work: ls docs/11-proposals/ | grep -i # Existing implementations -grep -r "" mdl/executor/ sdk/mpr/ --include="*.go" -l +grep -r "" mdl/executor/ mdl/backend/modelsdk/ --include="*.go" -l # Existing test coverage ls mdl-examples/doctype-tests/ | grep -i diff --git a/.claude/commands/mxcli-dev/review.md b/.claude/commands/mxcli-dev/review.md index 6f1145bf56..0d9124d4a1 100644 --- a/.claude/commands/mxcli-dev/review.md +++ b/.claude/commands/mxcli-dev/review.md @@ -51,6 +51,8 @@ proactively. Add a row after every review that surfaces something new. | 21 | A registry that reads only one source (an `embed.FS`) has no extension point, so the only way to customise its output is to hand-edit the generated block — which a digest fence then refuses to touch on the next run. The feature ends up hostile to the exact case it was built for | API design | When a guard refuses a user's edit, ask whether the thing being edited should have been authorable. Introduce a source abstraction (`fs.FS` + root) and let a project-local directory shadow the embedded set — the walk functions usually already take a root, so the change is contained | | 22 | A test helper that expands a template cross-products every list against every reference (e.g. every `@each $weight` list against every `url()`), inventing artefacts no asset ever shipped and failing on correct input | Test coverage | Scan positionally: a reference must be expanded against the loop it actually sits under. Prove the helper both fires on a real break (delete one shipped file) and stays quiet on correct input — a helper only checked against green code has not been shown to detect anything | | 23 | A copy-to-scaffold path renames files but not the identifiers built from the name (`@mixin mxcli--`, `@import "mxcli-"`), so two artefacts collide the moment both exist — and the symptom is a rule that silently compiles to nothing | Code correctness | Assert the structural contract on the *generated* artefact, not just the shipped ones: factor the built-in's contract test into a helper and run the scaffold through it. Verify once end to end against the real toolchain and record it | +| 24 | A skill or command still instructs work into a package the repo has deleted (`sdk/mpr` test locations, a symptom table moved to `findings/*.jsonl` years prior) — the doc reads as authoritative and every instruction in it is a compile error or a no-op | Docs quality | When a package is deleted or a doc is restructured, grep `.claude/` for its name in the same PR. A deletion that leaves the guidance behind is worse than no guidance, because the reader trusts it | +| 25 | A doc or skill shows a CLI invocation nobody ran — a command name that does not exist (`mxcli dump-bson` for `mxcli bson dump`), or a flag form the parser rejects (`--compare "A" "B"` where `--compare` is a StringSlice needing `"A,B"`). Worst when copied FROM the command's own `--help`, which had the same error, so the doc looks sourced | Docs quality | Run every command a doc shows, against a real project, before committing it. If it came from `--help`, run that form too — the example in the help text is not evidence that it works | --- diff --git a/.claude/lint-rules/orphaned_elements.star b/.claude/lint-rules/orphaned_elements.star index b8486e925f..39c04c01e8 100644 --- a/.claude/lint-rules/orphaned_elements.star +++ b/.claude/lint-rules/orphaned_elements.star @@ -28,7 +28,7 @@ ENTRY_PAGE_PATTERNS = ["Home", "Login", "Index", "Dashboard"] # Reference kinds that mean "something causes this microflow to run". These are # catalog RefKind values (mdl/catalog/builder_references.go); a kind missing here # turns a live document into a false "not called from anywhere" finding. -MICROFLOW_ENTRY_KINDS = ["call", "schedule", "datasource", "action", "calculate", "settings"] +MICROFLOW_ENTRY_KINDS = ["call", "schedule", "publish", "datasource", "action", "calculate", "settings", "event"] # Reference kinds that mean "something opens this page". PAGE_ENTRY_KINDS = ["show_page", "home_page", "login_page", "menu_item", "action"] diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 5bab3138ff..26093d2fa8 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -26,7 +26,15 @@ into failure classes; the findings are where you drill down for the instance. 5. **Prove the test detects the bug**: revert the fix and confirm it fails with the reported symptom. A test that has only ever run against fixed code has not been shown to detect anything. -6. After the fix: **append a finding** — see "Recording a fix" below. +6. **Build and run a real app if the fix's argument asserts what a Mendix tool + accepts or rejects** — see "Two rules the checklist above does not cover". + This fires on fixes step 4 does not catch: a version guard, a refusal, anything + justified by "mxbuild/Studio Pro would (not) accept this". +7. After the fix: **append a finding** — see "Recording a fix" below. + +> Before concluding that something **cannot be verified here**, read the second of +> those two rules. That claim ends the investigation, so it needs the evidence a +> fix would. > **Why the findings are not in this file.** They were, as one Markdown table, and > it reached 1.05 MB / 630 rows — past what fits in a context window and past what @@ -113,63 +121,100 @@ never read in order. ## TDD Protocol -Always follow this order — never implement before the test exists: +Never implement before the test exists: ``` -Step 1: write a failing unit test (parser test or formatter test) -Step 2: Confirm it fails to compile or fails at runtime -Step 3: Implement the minimum code to make it pass -Step 4: run: /c/users/Ylber.Sadiku/go/go/bin/go test ./mdl/executor/... ./sdk/mpr/... -Step 5: add the symptom row to the table above if not already present +Step 1: write a failing test at the layer the symptom lives in +Step 2: confirm it fails — and that it fails with the REPORTED symptom +Step 3: implement the minimum code to make it pass +Step 4: go test ./... (or the packages you touched) +Step 5: append a finding to findings/.jsonl ``` -Parser tests go in `sdk/mpr/parser__test.go`. -Formatter tests go in `mdl/executor/cmd__format__test.go`. +Where the test goes, by layer: + +| Layer | Package | +|-------|---------| +| grammar / parse tree | `mdl/visitor/` | +| BSON we encode or decode | `modelsdk/codec/`, `modelsdk/mpr/` | +| backend mutation | `mdl/backend/modelsdk/` | +| executor handler | `mdl/executor/` (with `MockBackend`) | +| version gating | `sdk/versions/` | + +`sdk/mpr` is **gone** — the legacy engine was deleted once its importer count +reached zero, so a test written there is a compile error rather than a wrong +answer. Earlier versions of this skill sent tests there. --- -## Issue #212 — Reference Fix (seeding example) - -**Symptom:** `describe microflow` showed `$var = list operation ...;` for -`microflows$find`, `microflows$filter`, `microflows$ListRange`. - -**Root cause:** `parseListOperation()` in `sdk/mpr/parser_microflow.go` had no -cases for these three BSON types — they fell to `default: return nil`. - -**Files changed:** -| File | Change | -|------|--------| -| `sdk/microflows/microflows_actions.go` | Added `FindByAttributeOperation`, `FilterByAttributeOperation`, `RangeOperation` | -| `sdk/mpr/parser_microflow.go` | Added 3 parser cases | -| `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | -| `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | -| `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | -| `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | -| `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | -| An import/export mapping over an entity created with `EXTENDS` maps only its **own** attributes; every inherited field shows unmapped in Studio Pro, and `mx check` reports CE1613 "The selected attribute 'Mod.Child.Attr' no longer exists". An inherited Boolean/DateTime element also gets `DataType=String` | The mapping builder prefixed the entity being mapped unconditionally (`attr = parentEntity + "." + attr`), but a member reference is qualified against the entity that **declares** it — the same rule as entity access rules (#758) and the change-object writer (#451). Separately `resolveAttributeType` scanned only the entity's own attributes and fell through to its `"String"` default | `mdl/executor/cmd_import_mappings.go` and `cmd_export_mappings.go` (both carry the same two lines), `mdl/executor/entity_hierarchy.go` (`ResolveMemberRef`, `ResolveMemberType`) | Route both sites through the generalization walk added for #758: `ResolveMemberRef` returns the declaring-entity reference and `ResolveMemberType` finds the type up the chain, each falling back to the old behaviour when the member cannot be resolved. **Watch for the sibling defect**: the old `resolveAttributeType` matched entities **by name across every domain model**, so a same-named entity in another module could win — resolve the module by name instead. **Generalisable**: when one rule has several call sites, a fix at one of them proves nothing about the others; grep for the *pattern* (`range entity.Attributes`, `parentEntity + "."`) rather than the reported symptom. Repro `mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl`; A/B on the same project shows `Map703.Contract.DocName` (CE1613) become `Map703.DocumentBase.DocName`. Issue #703, umbrella #765 | -| `alter settings model JavaVersion = 'Java21'` on Mendix 11.12+ produces a project mxbuild refuses to **load**: `mx check` reports `System.ArgumentOutOfRangeException ... (Parameter 'majorVersion is an unsupported value: Java21')` at `JavaVersionExtensions.fromString`. Every check downstream of the settings unit is lost with it | Mendix renamed the property between 11.6 (`JavaVersion` = `"Java21"`) and 11.12 (`JavaMajorVersion` = `"21"`) — and the rename changed the **value format** as well as the key. The #759 fix followed only the key, writing the caller's value through verbatim, so the 11.6 spelling landed in the 11.12 key | `mdl/settingsoverlay/settingsoverlay.go` (`JavaVersionValue`, `SetJavaVersion`) — shared by both engines; the dead third copy in `modelsdk/mpr/serialize_services.go` carried it too | Render the value in the dialect the stored key expects: strip/add the `Java` prefix per key, and pass an unrecognisable value through untouched so a typo surfaces as a Mendix error instead of a mangled setting. **Generalisable**: a renamed property is not only a renamed key — check whether the value encoding moved with it, and cover *both* directions (either spelling in, document's dialect out). Note the sharper failure mode: the original #759 shape was an unknown property, which mxbuild **tolerates**, so only Studio Pro broke; a wrong *value* for a known enum is a hard build failure, which is why this one surfaced as a red nightly rather than a user report. Repro `mdl-examples/bug-tests/759-java-version-value-dialect.mdl`. Issue #759 (follow-up) | -| On **Mendix 11.13 only**, every microflow using `EXECUTE DATABASE QUERY` fails `mx check` with **CE5277** "Please re-run and save the query to fix the error", once per activity. The queries themselves report nothing — the error lands on the *activities* pointing at them, so it reads like a microflow defect. Both engines. 11.12 and below are clean | 11.13 replaced the integer `QueryType` (1 = custom SQL) on `DatabaseConnector$DatabaseQuery` with a `Type` **string enum** (`Select` / `NonSelect` / `Unknown`), shipping a one-time conversion (`ExternalDatabaseConnectionQueryTypeConversion`) for old documents. mxcli wrote the legacy integer unconditionally, so on 11.13 the new property was simply **absent** — and an absent `Type` reads as Unknown, which is exactly what CE5277 reports | `mdl/dbconnector/querytype.go` (new, shared by both engines), `sdk/mpr/writer_dbconnection.go` + `parser_dbconnection.go`, `mdl/backend/modelsdk/db_write.go` + `integration_read.go`, `model/types.go` (`DatabaseQuery.QueryTypeName`) | Branch on the project's Mendix version (`ProjectVersion().IsAtLeast(11, 13)`) and write **exactly one** spelling. Writing both is not a safe hedge — a property the target's metamodel does not define is the #759 Studio-Pro-won't-open shape. Read side must accept either, or the next ALTER of an 11.13 project writes Unknown straight back. mxcli can't derive the type the way Studio Pro does (running the query and inspecting the result set), so it reads the leading SQL keyword — still better than Mendix's own converter, which marks every migrated query `Select` regardless of statement. **Diagnosis method**: `mx convert -p -s ` with the NEW mxbuild runs the version's own migration, then diff the BSON — that is what showed `QueryType: 1` → `Type: "Select"` without guessing. **Generalisable**: onboarding a new Mendix minor is not just adding it to the nightly matrix — run the doctype corpus against it first (`MX_BINARY=~/.mxcli/mxbuild//modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts`), because a renamed property surfaces as a red matrix job, not a compile error. Repro `mdl-examples/bug-tests/1113-database-query-type-enum.mdl`. Sibling drift found in the same sweep and deliberately NOT fixed: **CE5278** ("The JDBC driver is missing from the module settings"), a new 11.13 check about the module's Java dependencies, which mxcli has no way to author | -| A user reports CE0463 "the definition of this widget has changed" on DataGrid2 / Gallery / filters after upgrading the **Data Widgets** marketplace module. Reads like template drift; on real projects it is usually **not an mxcli bug at all** | A widget package that DROPS a property leaves every *stored* instance carrying a property the new definition lacks — which is precisely what CE0463 reports, and what its own message ("Update all widgets") tells you to fix. Ledger on 11.12: 0 errors at Data Widgets 3.4 (as authored) → 36 CE0463 at 3.11.3 → **0 again after `mx update-widgets`**. Single cause: `key="advanced"` is in `Datagrid.xml` at 3.4 and gone at 3.10/3.11 | no code change — diagnostic. See `docs/03-development/WIDGET_BSON_VERSION_COMPATIBILITY.md` "CE0463 after a widget-package upgrade" | **Two controls settle it, neither optional.** (1) Do **Studio Pro's own** widgets fail too? A blank project's `dataGrid2_*`/`gallery1,2`/`drop_downFilter1,2` are Mendix-authored — if they fail alongside mxcli's, the tool is not the variable (29 of Ledger's 36 were these). (2) Does **`mx update-widgets` clear it**? If yes, mxcli's BSON was structurally valid and correct for the version it was written against; genuine template bugs do NOT clear this way (the Image stale default and the number-filter markerless array both needed template fixes). **The real mxcli defect is the residue after those controls**: author FRESH against the new package (`widget init` + author + `mx check`) — on 3.10/3.11 that leaves DataGrid2 **clean** and only Gallery + DatagridDropdownFilter failing, i.e. far narrower than the issue as filed. **Trap that cost a full round-trip**: measuring with the doctype fixtures alone mixes both cases, because their pages live in a blank project whose own template widgets are already failing — subtract by widget NAME against a control project that ran no mxcli command. Issue #716 | -| Freshly authored **Gallery** widgets fail `mx check` with **CE0463** on Data Widgets 3.10+, while the 3.4 package bundled with Mendix 11.12/11.13 is clean — so it looks like ordinary post-upgrade staleness. Every schema-level explanation is disproven (property sets, list markers, ordering, pointer topology, `GenerateFromMPK`) | `syncDefinitionAttrs` reconciled a surviving property's definition attributes (`Required`, `OnChangeProperty`) from the installed `.mpk` onto the `CustomWidgets$WidgetPropertyType` node — but they live one level down, on its **`ValueType`**. The "only update a key that already exists" guard then never fired, making the entire pass a **silent no-op**, so the Gallery kept the embedded template's `OnChangeProperty = "onConfigurationChange"` where 3.10 expects `""` | `sdk/widgets/augment.go` + `modelsdk/widgets/augment.go` (`syncDefinitionAttrs`) | Descend to `ValueType` before the update (`getMapField(ptMap, "ValueType")`, falling back to the PropertyType). **Diagnosis method that found it**: diff against `mx update-widgets` output at the PATH level — the differing paths were `Type/ObjectType/PropertyTypes[N]/ValueType/OnChangeProperty`, and the path told me which node to write. **Generalisable — the trap that cost the most here**: a guarded update (`if _, ok := m[k]; ok`) aimed at the wrong node is *invisible*. It cannot fail loudly, so measurements read as "the fix didn't help" rather than "the fix never ran". When a change measurably does nothing, verify it executed before concluding the hypothesis was wrong. Result: fresh-authoring CE0463 on 3.10 went 6 → 2; bundled 3.4 stayed at 0. **Still open**: the two datagrid dropdown filters, whose residual diff is `ValueType/AllowUpload` (absent from the modelsdk template copy, present in sdk's — the engines' template sets have diverged); syncing them does not clear it. Repro `mdl-examples/bug-tests/716-widget-package-upgrade.mdl`. Issue #716 | -| Every authored **Data grid 2** fails `mx check` with **CE0463** on the *bundled* Data Widgets 3.4 (Mendix 11.12/11.13) — but only on the **legacy** engine; `modelsdk` is clean on the same script. Appeared the moment `syncDefinitionAttrs` was corrected to write to `ValueType` (the row above), i.e. the moment that pass first actually executed | The Mendix pluggable-widget XML schema defaults `required` to **true**; only an explicit `required="false"` is optional. `sdk/widgets/mpk` read a missing attribute as `false` (`p.Required == "true"`), so the now-live sync overwrote 24 correct `true`s with `false` on DataGrid2 3.4 (which omits `required=` on 24 of its 40 properties). `modelsdk/widgets/mpk` already read it correctly (`p.Required != "false"`, fixed under #600) — the engines had silently diverged on the default | `sdk/widgets/mpk/mpk.go` (all three `PropertyDef` construction sites: `walkPropertyGroup` top-level + nested-direct, `collectNestedProperties`) | Read `p.Required != "false"` so absent means true, matching `modelsdk` and `mx update-widgets`. **Generalisable — the shape to look for**: when two engines carry parallel copies of a parser, a fix applied to one leaves a *latent* divergence in the other that stays invisible until some unrelated change starts consuming the value. Grep the sibling package for the same expression before assuming a defect is engine-specific. **Diagnosis trap hit here**: an A/B ran out of disk mid-run, the exec silently created no widgets, and `mx check` reported 0 CE0463 — reading as "the pre-fix binary is clean". Always assert the artifact exists (`show widgets | grep `) before trusting a zero. Unit repro `sdk/widgets/mpk/required_default_test.go`; integration `TestMxCheck_DataGridPage`/`TestMxCheck_DataGridNoColumns`. Issue #716 | -| `mxcli widget sync` appears to succeed — project loads, CE0463 drops, `mprcontents/` survives — but afterwards **`mx update-widgets` cannot SAVE**: `System.InvalidOperationException: Duplicate Guid in unit page template '…'`. Worse, update-widgets collapses `mprcontents/` *before* it fails, leaving the project flattened AND unloadable (`Root unit not found`). A one-way door: sync silently forecloses the only complete remediation | `AugmentTemplate` adds a property to an object-list property (DataGrid2 `columns`) by giving **every list entry a copy of the same constructed node**, so one placeholder id appears N times. The sync's placeholder→UUID remap was keyed **by value**, so all N copies received the *same* fresh UUID. 3 added properties x 3 nodes each = 9 duplicated GUIDs per widget, x N columns — 18 units and 432 excess occurrences on the reference fixture. The template pipeline never hit it because a template has exactly one list entry | `mdl/executor/widget_convert.go` (`ensureUniqueWidgetIDs`, `widgetIDsAreUnique`) + `mdl/executor/widget_sync_apply.go` | Make `$ID` unique **per occurrence**, scoping the reference rewrite to the list entry: a `TypePointer` repeated across sibling entries is *correct* (every column's property points at the one shared `WidgetPropertyType`) and must not be rewritten. Then **refuse to write** any unit that still contains a duplicate. **Generalisable — the failure class**: Mendix validates GUID uniqueness only at SAVE, so `mx check` passing proves nothing about it; and the tool you would reach for to recover destroys the multi-file layout before discovering the problem. Any writer that duplicates nodes needs a pre-write uniqueness assertion, not a post-hoc check. **Testing trap hit here**: the unit test calls the fix directly, so it still passed with the call removed from the apply path — it proves the function, not the wiring. The pre-write guard is what actually caught the regression during the mutation check. Repro: run sync, then `python3` count duplicate `$ID`s per `.mxunit`. Reported against PR #89 | -| Freshly authored **drop-down filter** widgets fail `mx check` with **CE0463** on Data Widgets 3.10 while every other widget in the same script is clean. The stored BSON is missing `ValueType/AllowUpload` on all 25 ValueTypes and carries `Required=false` on `refCaption`/`refCaptionExp`, which the 3.10 XML declares `required="true"` — so the embedded template looks like the culprit, but replacing it changes nothing | `AugmentTemplate` never ran on this widget. Its opening guard — "nothing to add or remove at top level, and no nested children" — was written when the function only synced the property SET, and returns before the **six value-level passes** bolted on later (`reconcileEnumValues`, `reconcilePropertyMetadata`, `reconcileValueTypesFromMPK`, `completeValueTypeEnvelope`, `reorderPropertyTypes`, `syncDefinitionAttrs`). DataGrid2 never hit it because `columns` has nested children; the drop-down filter declares exactly the 25 keys the 11.6-era template already has, so it took the exit every time | `modelsdk/widgets/augment.go` + `sdk/widgets/augment.go` (`AugmentTemplate`) | Wrap the add/remove block in `if len(missing) > 0 \|\| len(stale) > 0 \|\| hasNestedChildren { … }` instead of returning early, so the value-level passes always run. **Generalisable — the shape to look for**: an early return placed correctly for a function's original job silently disables everything appended after it. When a pass "does nothing" for one input and works for others, check whether it *reached* the pass before theorising about the data (same root shape as the `ValueType` no-op two rows up). The `hasNestedChildren` clause in that guard is the tell: someone had already patched around the same bug for one widget rather than fixing it. **Diagnosis method**: a probe calling `augmentFromMPK` directly and counting `ValueTypes with AllowUpload` — 0/25 for the drop-down filter vs 44/44 for Gallery — localised it to "augment did not run" in one step, before any BSON theorising. Result: modelsdk on DW 3.10 went 2 → **0**. Tests `TestAugmentTemplate_MatchingKeysStillReconcilesValues` in both engines. Issue #716 | -| A **Decimal/DateTime** in `dynamictext` always rendered with the hardcoded default format ("5068.38000000"); no MDL way to set the per-parameter Format, and a widget-level `decimalPrecision:` was **silently dropped** (mxcli check ✓, exec ✓, but gone) | The model always stored `ClientTemplateParameter.FormattingInfo`, but **all three writers hardcoded it** (`DecimalPrecision:2, GroupDigits:false, DateFormat:Date, EnumFormat:Text`) and ignored `param.FormattingInfo`; the grammar had no syntax to set it and DESCRIBE dropped it on read too | grammar `mdl/grammar/domains/MDLPage.g4` (`paramAssignmentV3` + `paramFormatV3`), `mdl/ast/ast_page_v3.go` (`ParamFormatV3`), `mdl/visitor/visitor_page_v3.go` (`buildParamFormatV3`), builder `mdl/executor/cmd_pages_builder_v3_widgets.go` (`formattingInfoFromParamFormat`), writers `mdl/backend/modelsdk/widget_write.go` (`formattingInfoToGen`) + `sdk/mpr/writer_widgets.go` (`serializeClientTemplateParameter`), describe `cmd_pages_describe_output.go` (`formatParamFormatSuffix`), validate `validate_widgets.go` (`validateDynamicTextFormatting`/MDL-WIDGET18) | Add a per-param `FORMAT (decimalPrecision: N, groupDigits: bool, dateFormat: …, customDateFormat: '…', enumFormat: …)` block: `{1} = Amount format (decimalPrecision: 2, groupDigits: true)`. The **FORMAT keyword is required** — a bare `(…)` after the value is ambiguous with a function call because `:` is OQL division in expressions. Writers use the param's FormattingInfo when set, else the same hardcoded defaults (nil → byte-identical to before, zero risk to existing widgets). MDL-WIDGET18 turns a widget-level format key into an actionable error (no more silent drop) and validates keys/enums. Verified: exec → `mx check` (11.12.1) 0 errors + DESCRIBE round-trip. Repro `mdl-examples/bug-tests/ledger-75-dynamictext-formatting.mdl`. Ledger #75 | -| dynamic-text `format (…)` writes valid FormattingInfo, `mx check` ✓, but **renders unformatted** at runtime — a Decimal shows `-12` not `-12.00`, dates ignore the format | The parameter was serialized as `Expression: toString($currentObject/Attr)` (a non-String attribute was wrapped in `toString()`), and Mendix applies FormattingInfo **only to attribute-bound** params — an Expression param bypasses it. The BSON was valid but inert; `mx check` and DESCRIBE can't catch a *render* problem (the exact trap the runtime-verify skill exists for — I shipped #75 without it and the tester caught this) | `mdl/executor/cmd_pages_builder_v3.go` (`resolveTemplateAttributePathFull`, the bare-attribute branch) | Bind a bare non-String attribute as a structured **`AttributeRef`**, not a `toString()` Expression — the runtime then renders it through FormattingInfo, exactly as Studio Pro does. `toString()` was never required by mxbuild (AttributeRef for a Decimal/DateTime in a text template passes `mx check` → 0 errors). Engine read-parity holds. Note the `$param.Attr` non-String path (same function) still uses `toString()` — rarer, left for follow-up. Ledger #76 | -| A **DataGrid2 dynamic-text column** (`column x (ShowContentAs: dynamicText, Content: '{1}', ContentParams: [{1} = Attr format (…)])`) fails to open with **CE0463**, *and* its `format (…)` block is silently dropped. `mxcli docker check` hides the CE0463 because it runs `mx update-widgets` first; raw `mx check` and `mxbuild --serve` (run --local) surface it | Two independent gaps in the full-page object-list column path. (1) The shared `buildClientTemplateParams` never read the parsed `p.Format`, and the column-scoped serializer `SerializeColumnClientTemplateParameter` **hardcoded** FormattingInfo — so a column param's format was dropped at both write points. (2) A dynamic-text column has no attribute and no content widgets, so `detectObjectListItemKind` classified it as the **default** kind, which has no empty-ClientTemplate rules → its `tooltip` serialized as `TextTemplate:null`. Studio Pro stores an **empty `Forms$ClientTemplate`** there (as for an attribute column), so the widget failed to load | `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildClientTemplateParams` → apply `formattingInfoFromParamFormat(p.Format)`), `mdl/backend/widgetobj/builder.go` (`SerializeColumnClientTemplateParameter` honours `param.FormattingInfo`; new `itemKindDynamicText` + `emptyClientTemplateRules` tooltip entry; `detectObjectListItemKind`), describe `mdl/executor/cmd_pages_describe_pluggable.go` (`extractTextTemplateParameters` zips the format suffix) | Route the FORMAT block through the *shared* params helper (fixes the object-list column path **and** the ALTER PAGE column path at once) and stop hardcoding FormattingInfo in the column serializer. Classify a `showContentAs: dynamicText` column as its own item kind and give it the attribute column's `tooltip → empty CT` rule (exportValue stays null). **Diagnosis method that found the CE0463**: `update-widgets` on a *copy* cleared it → Case B (our BSON); a path-level flatten-diff of the datagrid subtree, mine vs the reconciled reference, isolated the single differing path `columns[dynamicText]/tooltip/TextTemplate` null↔empty. **Trap**: `docker check`'s built-in `update-widgets` masks the very defect you're hunting — measure with raw `mx check` or the serve build. Repro `mdl-examples/bug-tests/ledger-77-datagrid-dynamictext-column.mdl`; verified end-to-end (raw `mx check` 0 errors + Playwright cell renders `-1,234.50`). Ledger #77 | -| A `create json structure … snippet` produces a structure whose **import mappings silently import zero objects** — the REST call succeeds, the mapping looks right, and no data arrives. No validation error, `mx check` passes. Dumping the stored BSON shows every element at `MinOccurs=0, MaxOccurs=0` | Mendix reads `MaxOccurs=0` **literally as "never occurs"**, not as "unspecified". The snippet→element builder hardcoded `0/0` at all nine construction sites; `cmd_import_mappings.go` then copies MinOccurs/MaxOccurs straight onto the mapping elements, so the dead bound propagates from the structure into every mapping bound to it | `mdl/types/json_utils.go` (`BuildJsonElementsFromSnippet` + `buildElementFromRawObject` / `buildElementFromRawRootArray` / `buildElementFromRawArray` / `buildValueElement`) | Derive occurrences from the JSON shape: root `1..1`, Object/Value `0..1`, Array `0..1` with its **item** child `0..*` (`MaxOccurs = -1`), primitive-array Wrapper `0..*`. Added the named constant `occursUnbounded` so `-1` is not a bare literal. **Generalisable — the shape to look for**: when a tree comes out uniformly wrong except for *one* node, that node proves the writer is capable of the correct value and localises the bug to the construction sites that hardcode it — here the nested object-array item was already `0..-1` while the root-array item beside it was `0..0`, i.e. the same construct written two ways in two builders. **Trap**: `0` looks like a harmless default for a numeric field, so this reads as correct in review and survives every checker; only a BSON dump or a runtime import reveals it. **Follow-up that the first cut missed**: Mendix cross-validates every *mapping* element's occurrence against its bound *schema* element and reports **CE5015** ("Attribute 'MaxOccurs' does not match schema element") on a mismatch. Import writers already propagated occurrences; all **three** export writers (`modelsdk/mpr/serialize_mappings.go`, `sdk/mpr/writer_export_mapping.go`, `mdl/backend/modelsdk/mapping_write.go` — the last is the one the default engine actually uses) hardcoded `MaxOccurs: 0` on value elements, so raising the schema broke every export mapping. **Generalisable — the shape to look for**: changing a value that another document is validated *against* is never a one-sided edit; grep every writer that emits the same key, and expect more than one engine to have its own copy. **Verification trap that caused the miss**: the repro created JSON structures but no mapping bound to one, so it passed `mx check` while the integration suite went red — when a fix changes a field that other documents reference, the repro must instantiate a *referencing* document, not just the changed one. Repro `mdl-examples/bug-tests/841-json-structure-occurrences.mdl` (now includes an import+export mapping); verified end-to-end (`mx check` 11.13.0 0 errors on the repro and on both doctype scripts that failed CI). Issue #841 | -| A workflow **`DECISION`** whose expression uses the documented lowercase `$workflowContext` fails `mx check` with **`[error] [CE0117] "Error(s) in expression." at Decision 'Decision'`**, while the *same spelling* in a `CALL MICROFLOW … WITH` clause works. `mxcli check` and `mxcli exec` both report success | The context parameter is named `WorkflowContext` and Mendix expressions are case-sensitive on 11.9+, so `$workflowContext` is an undefined variable. `normalizeWorkflowContextExpr` existed and was well-tested, but was only *applied* in `autoBindCallMicroflow` (the FINDINGS #39 fix) — `buildExclusiveSplit` stored `n.Expression` verbatim. The working WITH clause is what disguised it: the user reasonably concludes the spelling is fine | `mdl/executor/cmd_workflows_write.go` (`buildExclusiveSplit`, and the sibling `buildWaitForTimer` whose delay may reference a context date attribute) | Run the authored expression through the existing `normalizeWorkflowContextExpr` at every site that accepts a user expression — there are three in the workflow writer, and only the parameter-mapping one was covered. **Generalisable — the shape to look for**: a *normalizer that exists and is unit-tested* is not evidence it is *called*; grep the call sites, not the helper. When one input spelling works and an identical one fails, compare the two code paths before questioning the data. **Also fix the docs that teach the broken form** — `.claude/skills/mendix/write-workflows.md` showed lowercase in its DECISION example and is synced into user projects by `mxcli init` via `cmd/mxcli/skills/`, so the bug propagated to every generated project. Repro `mdl-examples/bug-tests/845-workflow-decision-context-casing.mdl`; verified end-to-end (`mx check` 11.13.0: 1 error → 0). Issue #845 | - - -**Key insight:** `microflows$ListRange` stores offset/limit inside a nested -`CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before -extracting `OffsetExpression`/`LimitExpression`. +## Two rules the checklist above does not cover + +Both were paid for on mendixlabs/mxcli#1121, where everything else in this skill +was followed and the fix still shipped with a false claim in its PR body. + +### Run the real thing when the argument is about what a Mendix tool accepts + +`verify-in-runtime.md` asks whether the symptom is a property of the *running +app*. That is the right question for a rendering bug (#812) and it does not fire +here: #1121 was a version gate, and its unit tests were sound — they proved the +refusal was gone and the right keys were written. + +What they could not touch is the claim the fix *rests* on. The guard omits two +11.5-only properties below 11.5 because writing them is supposed to be unsafe. +Is it? Nothing in the test suite can say. Building a real 10.24.25 app with the +keys forced back in answers it in one run: + +``` +The app contains: 0 errors. +``` + +mxbuild accepts them silently — which is what makes the guard load-bearing +rather than decorative, and is not something any amount of reasoning establishes. + +**So: add a full run whenever the fix's justification asserts that a Mendix tool +would (or would not) catch something.** `mx check` tolerating a malformed +document is the most common shape of this, and it is exactly the case where a +green build is mistaken for evidence. Cheapest form: two copies of a real +project, one with the fix and one with the fault forced back in, `mxcli docker +check` on both. + +### "Cannot be verified here" is a claim, and needs the same evidence as a fix + +It is the claim that *ends* an investigation, so it gets the least scrutiny and +does the most damage. + +On #1121: `mxbuild-10.24.25.tar.gz` 404'd. So did a dozen sibling 9.x and 10.x +versions. 11.6.0, 11.12.1 and 11.13.0 all returned 200 from the same host and +the same path. The conclusion — Mendix 10 is not downloadable from this +environment — went into a PR body as fact. + +It was false. Mendix 9 and 10 publish **four-part** artifact names carrying a +build number: the real file is `mxbuild-10.24.25.122571.tar.gz`. One CDN listing +call (`?list-type=2&prefix=runtime/mxbuild-10.24.`) would have shown it, and did, +the moment someone asked the right question. `mxcli` now resolves this itself. + +The tell was in the shape of the evidence: **a negative that is uniform across an +entire class is evidence about the query, not about the class.** Nothing about +"Mendix 10 was withdrawn from the CDN" predicts that *every* 10.x patch fails +identically while every 11.x succeeds; a wrong filename predicts exactly that. + +Before writing "cannot": + +1. Say what a positive result would have looked like. +2. Check the probe could have produced one — try the thing you are sure works + (the 11.x control), and try a different *shape* of query, not another value. +3. If it still holds, state the limitation with the evidence attached, so the + next reader can attack it. Do not state it as a property of the environment. --- ## After Every Fix — Checklist -- [ ] Failing test written before implementation -- [ ] `go test ./mdl/executor/... ./sdk/mpr/...` passes +- [ ] Failing test written before implementation, at the layer the symptom lives in +- [ ] Test proven to detect the bug: revert the fix, confirm it fails with the reported symptom +- [ ] Full run done if the fix's argument asserts what a Mendix tool accepts or rejects +- [ ] Any "cannot be verified" claim carries its evidence, and a control that could have falsified it +- [ ] `make test && make lint` pass +- [ ] `mdl-examples/bug-tests/-.mdl` added for the regression case - [ ] New finding appended to `findings/.jsonl` (if not already covered), and `make check-findings` passes - [ ] PR title: `fix: ` diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index b26492ffbb..d5301655d9 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -111,3 +111,7 @@ {"area":"cmd/mxcli","date":"2026-09-15","symptom":"Porting the last cmd/mxcli readers off sdk/mpr, cmd_extract_templates.go compiled with a type error (RawType/RawObject are bson.D on sdk/mpr, any on types.RawCustomWidgetType). Casting past it would have compiled — and broken the command at runtime, because FindCustomWidgetType is UNIMPLEMENTED on the codec backend.","cause":"mdl/backend/modelsdk/unimplemented_gen.go carries FindCustomWidgetType; measured at runtime it returns 'FindCustomWidgetType is not implemented on the model engine. This should be unreachable'. cmd_extract_templates.go was calling it through a concrete *mpr.Reader, so it was reachable only by NOT going through the backend.","file":"cmd/mxcli/cmd_extract_templates.go","fix":"Left this one file on sdk/mpr with a comment saying why and what would fix it (implement FindCustomWidgetType on the codec backend), and ported the other five. cmd/mxcli is otherwise clean; importers 13 -> 8.","insight":"The type error was the lucky part. A compile error is the ONLY reason this did not ship as a runtime failure — the cast that silences it is one line, and nothing else would have objected. When a port hits a type mismatch at a backend boundary, check whether the backend method is implemented at all before reconciling the types: `grep -n '' mdl/backend/modelsdk/unimplemented_gen.go` answers it in one command, and a runtime probe (connect read-only, call it, log the error) confirms it in under a minute. Note the direction of the trap: the unimplemented method's own error says 'This should be unreachable', and porting a caller to the backend is precisely what MAKES it reachable — so the #477 census blind spot (callers holding a concrete reader are invisible) cuts both ways. Second, smaller measurement trap in the same slice: a baseline diff of `check --post-migration` showed 50 lines vanishing, which looked like a regression and was not — the FIRST run built and cached a catalog inside the project, so the second run reused it. Two binaries must each get their own fresh copy of the fixture, exactly as for a write port; a command that caches into the project directory makes consecutive runs non-independent even when nothing is being written on purpose."} {"area": "cmd/mxcli", "date": "2026-09-16", "symptom": "mendixlabs/mxcli#1103: a RETRIEVE with LIMIT inside a .test.mdl block was reported as `mismatched input 'LIMIT' expecting {GROUP_BY, SELECT, HAVING}` — the OQL follow set — on the statement `mxcli syntax microflow.retrieve` prints as its own example. The reporter concluded the test-block path routes microflow statements into the OQL parser.", "cause": "It does not. The generated MxTest.Test_* microflow parses fine (measured end-to-end against a real 11.6.6 project: the flow was created with the LIMIT intact). The message came from `mxcli check`/the LSP being pointed at the .test.mdl file itself, which they parsed as top-level MDL. A test block is a MICROFLOW BODY: DECLARE is not a top-level statement, the parser resyncs, RETRIEVE is a NON-RESERVED keyword so it is swallowed as an identifier, and the leftover `FROM …` starts oqlQueryTerm's FROM-first alternative (mdl/grammar/domains/MDLCatalog.g4), whose follow set is exactly {GROUP_BY, SELECT, HAVING}.", "file": "cmd/mxcli/testrunner/check_source.go", "fix": "testrunner.CheckSource renders each block as the microflow it becomes, padded so every body keeps its SOURCE line numbers (wrapper fragments go on the lines the doc comment and the '/' separator occupied). cmd_check.go and lsp_diagnostics.go translate before parsing, so all downstream rules apply unchanged and no diagnostic needs remapping. .test.mdl files joined `make check-mdl`; `.fail.test.mdl` names one whose annotations are deliberately unusable.", "insight": "Two lessons. First: the reporter's diagnosis was precise, confident and wrong, and the fastest way to find that out was to run the pipeline rather than read it — dumping GenerateTestFlows' output and feeding it to visitor.Build took one throwaway test and settled in seconds what an hour of grepping had not. Their error message was real; the command that produced it was not the one they named. Second, the general shape: a tool that OWNS a file format must not hand that format to a parser for a different one. The VS Code extension binds MDL to `.mdl`, which `.test.mdl` matches, so every test file in the editor was a wall of squiggles — 9 of this repo's 10 test files reported errors, one of them 392, and nobody had noticed because nobody runs `mxcli check` on a test file. When adding a derived file format, check what the EXISTING tooling makes of it; the answer is rarely 'nothing'. Line-preserving padding is what makes the translation honest: render into a slice of the source's own length and place wrapper fragments only on lines the original spent on comments or separators, and a diagnostic's line:col is the author's without a mapping table to drift."} {"area": "cmd/mxcli", "date": "2026-09-16", "symptom": "mendixlabs/mxcli#1104: `mxcli test --attach` reported only 'build failed: The project cannot be deployed, because it contains errors.' on an injection failure, and afterwards EVERY later run of ANY test file failed the same way until a leftover document was found by hand.", "cause": "Two independent defects. (1) The parsed problems were in hand and discarded: runner_attach.go and LocalApp.Rebuild both built their error with `fmt.Errorf(\"build failed: %s\", build.Message)`, and Message is identical for every failing build. Attribution (build_attribution.go, BuildResult.ErrorSummary) existed but was wired only into the --local BOOT, so --attach and every --watch rebuild lost it. (2) Generated names are positional — MxTest.Test_test_1, _2, … from the test's index in its file — and every test file reuses them, while cleanup dropped only the CURRENT suite's names. A run with fewer tests than the last one therefore left the surplus behind, and under --attach the MxTest module always pre-exists (the dev loop installed it) so the whole-module drop never fires.", "file": "cmd/mxcli/testrunner/cleanup_leftovers.go", "fix": "buildFailure()/resultsForBuildFailure() shared by both runners; cleanup keys on what the project HOLDS (SHOW MICROFLOWS IN MxTest, filtered by the generated prefix) with the suite only as a fallback; reportCleanup names every surviving document and prints its DROP.", "insight": "Measured, not reasoned: planted one bad MxTest.Test_test_2 in a project, ran a known-good one-test suite, and watched it fail and leave the leftover in place — then after the fix watched run 1 fail and CLEAN, and run 2 pass. A self-healing sequence is the control that 'cleanup works' cannot be argued into. The general rule for generated artefacts: derive what to remove from what EXISTS, never from what you intended to create. Keying on the suite was wrong in both directions at once — it missed leftovers AND issued DROPs for flows a part-way injection never created, and those failures made cleanup report 'the project has been left modified' for a project it had just cleaned, which is a false alarm that sends the reader hunting for damage. Positional names (index-in-file) guarantee collisions across files and are worth avoiding, but as long as they exist the prefix is the only safe key. Also worth pinning: once BuildFailedError.Error() renders the errors, a hint that repeats them prints everything twice — assert on the message the READER sees, not on the hint in isolation."} +{"area": "cmd/mxcli", "date": "2026-09-17", "symptom": "On macOS with the matching Studio Pro installed, `run --local` and `test --local` fail in the build step: either `Project version '11.12.2' does not exactly match MxBuild version '11.14.0'` or `mxbuild at ... is a linux binary and cannot run on darwin`. `mxcli setup mxbuild` prints Studio Pro's path and changes nothing, so the remedy every error suggests is a loop", "cause": "Two resolvers answered 'which mxbuild', and the wrong one won. The local entry points call `ResolveMxBuildForLocal` (#916) and then pass the result ONLY to the web-client bundler; both `ServeOptions` literals omitted `MxBuildPath`, so `StartServe` re-resolved from `~/.mxcli/mxbuild/` and, on a miss, took `AnyCachedMxBuildPath()` \u2014 the NEWEST cached version, unrelated to the project's. `test --local` was worse: it never resolved at all, still calling `DownloadMxBuild` directly, the exact call #916 removed from `runlocal.go` and left in `localapp.go`", "file": "`cmd/mxcli/docker/mxserve.go` (new: `serveOptionsFor`, `resolveServeMxBuild`, `noServeMxBuildError`) + `runlocal.go` + `localapp.go` (the two ServeOptions literals)", "insight": "The half-populated cache directory that defeats `CachedMxBuildPath` is **mxcli's own work, two steps earlier in the same command**: `ensureMxBuildRuntimeSibling` does `MkdirAll` on `~/.mxcli/mxbuild//` to plant the runtime symlink, and on macOS nothing ever puts a `modeler/` beside it because mxbuild comes from Studio Pro. So this is not a user with a damaged cache \u2014 it is the guaranteed state for every macOS project whose version was not already downloaded, which is why it reads as 'run --local has never worked here'. Repro without a Mac: plant a cache entry for one version and call `StartServe` asking for another. **The control matters more than usual**: asserting only that the call FAILS proves nothing, because the old code also failed \u2014 one exec later, with `exec format error` \u2014 so the assertion has to be that the refusal names the missing version and happens before any exec. A first draft of the modeler-less test passed against unfixed code for exactly that reason. Fix shape, per docs-wiki/bug-patterns/duplicate-resolver-drift.md: remove the second answer rather than synchronise them \u2014 one `serveOptionsFor` both call sites go through, and a resolver that refuses to substitute a near-miss version instead of handing mxbuild a binary it will reject minutes later, after a cold model load. Issues #1122, #1124", "refs": ["#1122", "#1124", "#916"]} +{"area": "cmd/mxcli", "date": "2026-09-17", "symptom": "`mxcli run --local --mxbuild-path /x -p app.mpr` answers `Error: unknown flag: --mxbuild-path`, while the shipped run-local skill, runlocal.go's own comment and two resolution error messages all tell the user to pass it", "cause": "The flag was never registered on `runCmd`; it exists only on the four `docker` subcommands. Everything BEHIND it was already wired \u2014 `LocalRunOptions.MxBuildPath` is declared and `ResolveMxBuildForLocal` honours it \u2014 so the gap was one missing `Flags().String` and one missing field assignment, invisible to every test because nothing exercised the command's flag set", "file": "`cmd/mxcli/cmd_run.go` (flag registration + LocalRunOptions.MxBuildPath) + `.claude/skills/mendix/run-local/SKILL.md` + `docs-site/src/tools/run-local.md`", "insight": "The interesting defect is not the missing flag, it is that **the error messages recommending it were the only documentation of it** \u2014 guidance naming an option the command does not accept is worse than no guidance, because it reads to the user as their own mistake, and on macOS it was the only advertised way out of a platform mismatch. The regression test to write is therefore not 'the flag exists' but the invariant: scan the resolution sources for `--flag` strings they tell users to pass, and assert `run` registers each one (`TestErrorGuidanceNamesAFlagThatExists`). Watch the parse test \u2014 `-p` is PERSISTENT on rootCmd, so `runCmd.Flags().Parse` rejects it with `unknown shorthand flag: 'p'` and the test fails for a reason unrelated to the fix; resolve through `rootCmd.Find` to reproduce the reporter's line honestly. Issue #1125", "refs": ["#1125", "#916", "#1122"]} +{"area": "cmd/mxcli", "date": "2026-09-17", "symptom": "`mxcli run --local` on a project with Settings > Web UI > OptimizedClient = No exits 1 after the cold build with `no rollup.config.mjs and no bundle at .../web/dist/index.js ... the build did not produce a client` \u2014 about a deployment whose client is sitting in the same directory. `mxcli docker run` works", "cause": "The client gate tested for exactly two shapes (a rollup config to run, or a bundle mxbuild already wrote) and treated everything else as a failed build. A classic (Dojo) deployment is a legitimate third shape with NEITHER: there is no bundling step for the classic client in any Mendix version. Nothing in the local loop read UseOptimizedClient, so the mode was invisible", "file": "`cmd/mxcli/docker/webclient_plan.go` (new: `planWebClient`, `isClassicWebClient`, `noWebClientError`) + `webclient.go` (`BuildWebClient`, `ensureWebClientBundle`) + `webclient_watch.go` (`StartWebClientWatch`) + `runlocal.go` (`ensureClientServed`)", "insight": "Measured on one blank 11.12.2 app built twice, changing only the setting: **mxbuild swaps which client lands in `deployment/web/` and parks the other beside it** \u2014 OptimizedClient=Yes gives `web/` the React client (+`rollup.config.mjs`) and parks Dojo in `dojo-web/`; =No gives `web/` the Dojo client (`index.html` loading `mxclientsystem/mxui/mxui.js`, no rollup config, no dist) and parks React in `react-web/` WITH its own rollup config. So detect from the DEPLOYMENT, not from the model's setting: the deployment is what gets served, the two disagree exactly when the setting has just changed, it needs no plumbing through the five call sites, and it covers MigrationMode without predicting what that mode emits. Detect on POSITIVE evidence (the entry point names its client) \u2014 inferring classic from the absence of the React shapes would make every genuinely broken deployment look classic and silently skip the bundle, which is the black screen the gate exists to prevent; keep a control test that a clientless deployment still fails. **The gate had FIVE consumers, not one**: boot, the `--watch` bundler, the post-boot re-bundle guard, and `ensureClientServed`, which probes that `/dist/index.js` is *served* \u2014 measured 404 on a classic app, so fixing only the boot moves the failure to every applied change under `--watch`. Two of them carried hand-copied duplicates of the same gate and had already drifted once (the 11.14 fix, ako/mxcli-ledger #146, landed on `BuildWebClient` only, so `run --local` worked on 11.14 and `run --local --watch` did not) \u2014 so the fix collapses them into one `planWebClient` rather than adding a third copy. Repro from Linux with no Mac and no Studio Pro: `mxcli new` an 11.12.2 app, flip `UseOptimizedClient` to `No` on `Forms$WebUIProjectSettingsPart` (note the `Forms$` prefix, not `Settings$`), build, and keep the unflipped copy as the control. Patching that BSON with a Go `map` corrupts the file \u2014 mxbuild refuses it with `Expected '$ID' as the first property of a storage object` \u2014 because map iteration loses key order; use `bson.D` throughout. Verified in a browser, not just at the gate: `mx` global present, real page content, zero console errors. Issue #1123", "refs": ["#1123", "ako/mxcli-ledger#146"]} +{"area": "cmd-mxcli", "date": "2026-09-17", "symptom": "`mxcli new --version 10.24.25` (and `mxcli setup mxbuild --version 10.24.25`) dies with `HTTP 404 from https://cdn.mendix.com/runtime/mxbuild-10.24.25.tar.gz`. Every 9.x and 10.x version probed 404s while 11.6.0/11.12.1/11.13.0 return 200 from the same host and path, which reads as 'Mendix 10 is no longer on the CDN'.", "cause": "Mendix 9 and 10 publish FOUR-part artifact names carrying a build number the release notes never mention: the release called 10.24.25 is `mxbuild-10.24.25.122571.tar.gz`. Mendix 11 publishes three parts. `MxBuildCDNURL` interpolates whatever string it is handed and nothing resolved a partial version, so a hand-typed 10.x version named no artifact at all. Project-driven paths were never affected — the MPR's `_ProductVersion` already carries all four parts (`10.24.25.122571`) and `parseVersion` takes the first three for major/minor/patch while the full string goes to the URL.", "file": "`cmd/mxcli/docker/version_resolve.go` (ResolveCDNVersion, highestBuild, CDNReleasesFor); wired at the two entry points where a user types a version, `cmd/mxcli/cmd_new.go` and `cmd/mxcli/setup.go`. Tests `cmd/mxcli/docker/version_resolve_test.go`.", "insight": "**A uniform 404 across a whole major version is evidence about the NAME, not about availability.** The conclusion drawn from it — 'Mendix 10 cannot be downloaded here' — blocked a verification for an entire session, and the fix was one listing call: the CDN is an S3 bucket that answers ListObjectsV2 (`?list-type=2&prefix=runtime/mxbuild-10.24.`), so what exists is enumerable rather than guessable. When a probe fails identically for every input in a class, question the query before concluding the class is empty. Three traps in the resolution itself, each a test: the `.sha256` sidecar beside every archive must not be picked as an artifact; the prefix needs its trailing dot or `10.24.2` swallows `10.24.20`..`10.24.26`; and build numbers are not zero-padded, so a text sort puts 99999 above 122571 and 10.24.9 above 10.24.26. Resolve at the entry point and thread the RESOLVED string onward — `mxcli new` checks the created project's stamp against the requested version, and `mx create-project` stamps four parts, so resolving late would fail that postcondition.", "refs": ["#1121"]} diff --git a/.claude/skills/fix-issue/findings/mdl-backend.jsonl b/.claude/skills/fix-issue/findings/mdl-backend.jsonl index 3135561123..e448128210 100644 --- a/.claude/skills/fix-issue/findings/mdl-backend.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-backend.jsonl @@ -113,3 +113,6 @@ {"area":"mdl/backend","date":"2026-09-15","symptom":"Phase 4a took sdk/mpr from 27 importers to 0, but nothing stopped the count from creeping back — there was no build or test guard, only the plan document and a habit.","cause":"The invariant lived in prose. A single new `import \"github.com/mendixlabs/mxcli/sdk/mpr\"` compiles, passes every test, and reintroduces exactly the blind spot Phase 4a existed to close: the unimplemented-method census in mdl/backend/modelsdk lists methods with NO implementation, so a caller reaching one through a concrete *sdk/mpr.Reader never appears in it. That is what hid project_tree.go's 36 semantic reads (#477) and cmd_extract_templates.go's FindCustomWidgetType (#484) until each was found by hand.","file":"mdl/backend/sdkmpr_import_guard_test.go","fix":"TestNothingImportsTheLegacyEngine parses every .go file's imports (go/parser, ImportsOnly) and fails naming any file that imports sdk/mpr, with the remedy in the message. Controlled by dropping a one-line file importing sdk/mpr into examples/ — it fails and names the file.","insight":"A zero-count invariant needs TWO positive controls or it passes vacuously forever, and the failure mode is silent by construction: a walk rooted at the wrong directory, a skipped-dir rule that is too broad, or an import-parsing mistake all report '0 importers' and read as success. So assert (1) a plausible number of files was actually scanned (here >500; it sees 2551) and (2) the detector can see imports AT ALL, by counting a package the repo definitely does import (mdl/backend, 120 files). Only then does 0 mean zero. This is scripts/check-tunnel-deps.sh's pattern — it asserts chisel IS in the linux graph before asserting it is absent from windows/darwin — and the same reasoning as a bug-fix control: a test that only ever passes has not been shown to detect anything. Practical note: skip sdk/mpr's own directory by comparing the path to the repo root rather than by basename, or a directory named mpr elsewhere is skipped too."} {"area": "mdl/backend/modelsdk", "date": "2026-09-16", "symptom": "`describe enumeration System.WorkflowActivityType` -> \"enumeration not found\" while `describe entity System.WorkflowActivityRecord` prints `ActivityType: Enumeration(System.WorkflowActivityType)` in the same session. `show enumerations` omits every System enum; `check --references` REJECTS a valid attribute typed against one (a false positive that blocks correct scripts); `CATALOG.attributes LEFT JOIN CATALOG.enumerations` resolves 0 of 19 on a blank app. Values could only be guessed at until the build rejected one with CE1613", "cause": "`modelsdk/meta.SystemEnumerations` — all 15 System enumerations with their values — had been in the tree since #889 with ZERO non-test consumers. The System module is not stored in the .mpr at all (measured: the string `WorkflowActivityType` occurs in 0 of 370 mprcontents units and 0 bytes of the .mpr sqlite), so its entities/associations/Java actions are each synthesized by a `Build*` helper and appended to a listing. The enumeration half had the data table and neither the helper nor the wiring, so the entity attribute printed a type naming a document nothing could produce", "file": "`modelsdk/meta/system_enumerations.go` (new, `BuildSystemEnumerations`); wired in `mdl/backend/modelsdk/enumeration.go` (`ListEnumerations`, `GetEnumeration`)", "insight": "**A data table with no consumer looks exactly like a missing feature.** Before opening any file, `grep -rn --include=*.go | grep -v _test` — zero hits is the whole diagnosis, and it took one command. The virtual System module has one wiring point PER LISTING, so the question for any new System doctype is \"which listings must it appear in\", not \"is the data there\". **One append fixed four of the five reported symptoms at once** (describe, show, check --references, catalog) because the catalog builder takes `ctx.Backend` as its reader — so `ListEnumerations` is the single choke point. Two traps. (1) `search` was NOT fixed by it: the strings index is built from value CAPTIONS (`builder_modules.go`), and the meta table carries names only, so System enums produce 0 rows in `CATALOG.strings` while user enums produce 36. Defaulting a caption to the value name would be inventing text indistinguishable from a developer's own — left unfixed and filed instead. (2) A stale `.mxcli/catalog.db` made the catalog look unfixed for three measurements; delete it before concluding anything about catalog output. Diagnosis tip: the sibling guard `TestModelerSystemEntities_HaveResolvableGeneralizations` existed and its enumeration twin did not — when one half of a synthesized module has a resolvability test, check whether the others do", "refs": "mendixlabs/mxcli#1102, mendixlabs/mxcli#1071, #889", "ce": "CE1613"} {"area":"mdl/backend","date":"2026-09-16","symptom":"With sdk/mpr at zero importers, `rm -rf sdk/mpr` still would not have been safe: it had two live dependencies that an import-based check cannot see. sdk/mpr/version has six importers (two of them shipping code, cmd/mxcli/docker/build.go and patch.go), and cmd/mxcli/docker/update_widgets_test.go reads sdk/mpr/testdata/v1-project by FILESYSTEM PATH.","cause":"The zero-importer guard matched the exact string \"github.com/mendixlabs/mxcli/sdk/mpr\". A SUBPACKAGE is a different import path, and a testdata directory is not an import at all — it is an os.DirFS string. Neither shape appears in a check written against the parent package's path.","file":"sdk/mpr","fix":"Repointed the six version importers at mdl/types (sdk/mpr/version.ProjectVersion is `type ProjectVersion = types.ProjectVersion`, an ALIAS, so it is the same type rather than a compatible one — modelsdk/mpr/version declares a duplicate struct and would NOT have been), moved the v1-project fixture to modelsdk/mpr/testdata/, verified both with the package still present, and only then deleted. 163 files, 41,674 lines.","insight":"Before deleting a package, search for THREE things, not one: the package's own import path, its subpackages' import paths (`.../pkg/`), and its directory as a literal string (testdata read through os.DirFS, go:embed, scripts). The last two are invisible to any importer census. Repoint everything FIRST and prove the build and tests green while the package still exists — that separates 'the repoint was wrong' from 'the deletion was wrong', which a single combined commit cannot distinguish. Two measurements worth keeping: the shipped binary is byte-identical in SIZE before and after, confirming the linker had already dropped the package, so this deletion removes source weight and not runtime behaviour; and `sdk/widgets` dropped to zero importers as a side effect but must NOT be deleted, because modelsdk/widgets/dirty_template_test.go reads sdk/widgets/templates/mendix-11.6 by path — the same path-not-import trap, found by grepping for the directory name rather than the import."} +{"area": "mdl/backend", "date": "2026-09-17", "symptom": "A microflow's URL (the deep link Studio Pro shows on the microflow's properties, Mendix 10.6+, e.g. `item/{Key}`) disappears after any `CREATE OR MODIFY MICROFLOW` \u2014 including one that only edits the body. `mxcli check`, `mx check` and mxbuild all report success before and after; the loss is visible only in Studio Pro (#1120)", "cause": "`microflowToGen` wrote `out.SetUrl(\"\")` and `out.SetUrlSearchParametersQualifiedNames(nil)` unconditionally in its `major >= 10` block, `microflowFromGen` never read either back, and `sdk/microflows.Microflow` had no field to hold them \u2014 so the value had no path across a rewrite at any of the three layers", "file": "`mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `mdl/executor/cmd_microflows_build.go`, `sdk/microflows/microflows.go`", "fix": "Carry `Url`/`UrlSearchParameters` the way AllowConcurrentExecution/MarkAsUsed/ApplyEntityAccess already are: field on the semantic microflow, read in microflowFromGen, written from the model in microflowToGen, and seeded from the stored microflow in the executor's rewrite path. DESCRIBE emits a `-- URL: \u2026` note, because a describe -> rename -> exec COPY still has nothing to preserve from", "insight": "This is the fifth property in `microflows.Microflow` lost this way and the first with NO checker behind it, which is what made it a user report rather than an internal find. The earlier four were all caught by a build error eventually (CE4899 for the concurrency flags, CE0122 for Excluded) or by a security review (ApplyEntityAccess); a microflow with no URL is simply a valid microflow, so every gate stays green and only a human opening Studio Pro can see it. **Generalisable**: when auditing a rebuild for guard-don't-drop, rank the constants it writes by whether a checker would notice their absence \u2014 the ones nothing checks are the ones that reach users, and they are exactly the ones a 'does this look like configuration?' audit skips. The mechanical version is to diff a Studio Pro document key by key against the writer's output; here `grep 'out.Set.*(\"\")\\|(nil)' microflow_write.go` finds the whole remaining set in one line (`ExportLevel` pinned to \"Hidden\", `ConcurrencyErrorMicroflow`/`ConcurrencyErrorMessage` emptied \u2014 both still unguarded, though CE4899 makes the concurrency pair loud). Measured control: reverting either half (SetUrl or the read) alone fails TestMicroflowRoundTrip_DeepLinkURL with the reported symptom, so both halves are load-bearing"} +{"area": "mdl/backend", "date": "2026-09-17", "symptom": "A microflow's **export level** (Studio Pro's Hidden/API switch \u2014 whether it is part of the module's public surface when the module is exported as a package) is reset to `Hidden` by any `CREATE OR MODIFY MICROFLOW`. Every checker stays green, because a hidden microflow is a valid microflow; the module's API is simply smaller", "cause": "`microflowToGen` wrote `out.SetExportLevel(\"Hidden\")` unconditionally, `microflowFromGen` never read it back, and `sdk/microflows.Microflow` had no field \u2014 the identical three-layer gap as the deep-link URL in the same function", "file": "`mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `mdl/executor/cmd_microflows_build.go`, `sdk/microflows/microflows.go`", "fix": "Same carry as the URL, plus a DEFAULT: `\"\"` is not a member of `MicroflowsExportLevel`, so an empty model value is written as `Hidden` rather than passed through (the precedent is `json_write.go`). DESCRIBE emits `-- Export level:` only when the value is not `Hidden`", "insight": "Found by running the mechanical audit the URL fix prompted \u2014 `grep 'out.Set.*(\"\\|(nil)' microflow_write.go` over the one function \u2014 which is the cheap move after any instance of this class and turned up three more constants in one line. **The measurement that shaped the fix**: three real marketplace modules (Business Events 3.12.0, External Database Connector 6.2.3/6.3.0) store `Hidden` on 3 of 3 microflows and 55 of 55 documents overall, all three exporting at module level `Source` \u2014 so the hardcoded value was not wrong, it was a default masquerading as a constant. That is the shape of the trap: the audit finds the constant, but only a reference document tells you whether to carry it, default it, or leave it alone. A marketplace `.mpk` is a free source of these \u2014 `unzip -o pkg.mpk project.mpr` gives a real Studio Pro-authored MPR to query, no Studio Pro and no network needed (`mx-modules/` holds three). **Never carry an enum-valued property straight through without a default**: a stored document that says nothing reads as `\"\"`, and writing `\"\"` back is precisely the unloadable-model write CLAUDE.md warns about \u2014 mxbuild tolerates it and Studio Pro throws at MprProperty.cs. Controls: pinning the writer back, stubbing the reader, and neutralising the executor carry each fail a different test with the reported symptom"} +{"area": "mdl/backend", "date": "2026-09-17", "symptom": "Unit tests for a carried microflow property (URL, export level, concurrency) all pass, and the end-to-end behaviour against a real project is still unverified \u2014 the integration gate that would have caught it, `TestMxCheck_DoctypeScripts`, `t.Skip`s whenever `mx` is absent, which is every run in a fresh container", "cause": "Two separate measurement errors, both invisible to `go test`. (1) The test fixture paired `Url: \"item/{Key}\"` with `UrlSearchParameters: [\"\u2026.Key\"]` \u2014 the SAME parameter \u2014 which mxbuild rejects as **CE5612**: a parameter used in the URL path may not also be a search parameter. Nothing in a unit test validates the model, so the fixture described a document Mendix refuses to build. (2) `bin/mxcli` was stale: `go build ./mdl/...` and `make test` had been run after each fix, but not `make build`, so the end-to-end run exercised a binary predating two of the three commits", "file": "`mdl/backend/modelsdk/microflow_roundtrip_flags_test.go`, `mdl/executor/microflow_carried_properties_test.go`, `mdl/executor/roundtrip_doctype_test.go` (the skipping gate)", "fix": "Fixture uses a distinct `Filter` parameter and says why. End-to-end procedure that actually measures it: `mxcli setup mxbuild -p ` (~719 MB, works through the session proxy), copy `testdata/expr-checker` as the fixture, create the microflow with mxcli, seed the unauthorable properties straight into the stored unit with `mpr.NewWriter` + `UpdateRawUnit`, then `mx check` BEFORE (the fixture must be a document Mendix accepts, or it proves nothing), `mxcli exec` a body-only rewrite, read the unit back, `mx check` after", "insight": "**A skipping integration gate is worse than no gate**: `mxCheckAvailable()` + `t.Skip` means a green `make test` says nothing about mxbuild, and reading the CLAUDE.md line about #808 is not the same as checking whether it applies to your own run \u2014 `ls ~/.mxcli/mxbuild` is. **Rebuild the binary before any end-to-end run**, and check its mtime against the last commit: a stale `bin/mxcli` produced a result (URL survived, export level did not) that looked exactly like a genuine second-read-path defect, and sent me hunting for a duplicate resolver that does not exist. **Seed the fixture through the writer, not by hand-editing BSON**, and always `mx check` the seeded state first: the CE5612 error came from the seed, not from mxcli, and without the before-check it would have been misattributed to the fix. Measured, mxbuild 11.6.6: pre-fix binary rewrites the microflow to `Url=\"\"`, empty search params, `ExportLevel=\"Hidden\"`; post-fix keeps all three; `mx check` 0 errors on both the seeded control and the rewritten project"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 273d4d607e..fdf81448e0 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -636,3 +636,4 @@ {"area": "mdl/executor", "date": "2026-09-16", "symptom": "`checkbox cb (Attribute: A, Editable: Never, ReadOnlyStyle: Control)` parses, passes `mxcli check`, is reported as executed \u2014 and the stored document keeps ReadOnlyStyle \"Inherit\". DESCRIBE *does* read and emit the property, so describe -> exec on a Studio Pro-authored page silently downgraded Control to Inherit. Visible only in the browser: with Inherit a read-only check box renders the TEXT \"Yes\"/\"No\", with Control the (disabled) checkbox glyph \u2014 measured on 11.6.6 by patching the stored string by hand ('Inherit' and 'Control' are both 7 bytes, so a byte substitution in the .mxunit is a valid document; mx check 0 errors both ways).", "cause": "Three silent layers. sdk/pages.CheckBox had no ReadOnlyStyle field, so buildCheckBoxV3 had nowhere to put the property; the codec writer hardcoded g.SetReadOnlyStyle(\"Inherit\") (and a per-type constant for TextBox/TextArea/DatePicker/RadioButtons, \"Control\" for DataView); and `ReadOnlyStyle` sits in validate_widgets.go's known-property allowlist under the comment \"vocabulary describe page emits\", which silenced the MDL-WIDGET check that flags a property no builder consumes.", "file": "mdl/executor/cmd_pages_builder_v3_widgets.go", "fix": "Add ReadOnlyStyle to pages.CheckBox; read and canonicalise it in buildCheckBoxV3 (Inherit/Control/Text, case-insensitive in, Mendix casing out, unknown value refused); write orDefaultStr(x.ReadOnlyStyle, \"Inherit\") in the codec so an omitted property still produces the document it always did. ako/mxcli#490.", "insight": "An allowlist added so DESCRIBE output re-parses will also silence the warning that a property is going nowhere \u2014 the two uses are indistinguishable from inside the checker, so entries added for the first reason need a consuming builder or they become a licence to drop. The asymmetry is the tell: a describer that READS a property whose writer hardcodes a constant is a round trip that quietly rewrites the user's model, and it is worth grepping for that pairing directly (`grep SetX(\"literal\")` against what extract*/describe reads). Measurement trick worth reusing: when the model layer cannot express a value, patch the stored BSON to the candidate value and boot \u2014 a same-length string substitution keeps the document valid, which settles 'is this the property that changes the rendering?' before writing any Go. The end-to-end control afterwards is the elision verb: re-authoring through MDL over the hand-patched document reported `Unchanged page`, i.e. what mxcli now writes is byte-for-byte the document that was verified in the browser."} {"area": "mdl/executor", "date": "2026-09-16", "symptom": "A .def.json that maps a Data Grid 2 column filter's `linkedDs` produces a widget mxbuild rejects with CE0642 \"Property 'Datasource to Filter' is required\" — naming the very property the value was written into", "cause": "`linkedDs` is declared `isLinked=\"true\"` in widget.xml: the platform fills it from the containing DataGrid2, and mxbuild resolves it from the parent rather than reading what is stored. mxcli could not tell a linked datasource from an authorable one because IsLinked, though present in the template ValueType, was not carried into PropertyTypeIDEntry", "file": "`mdl/types/widget_property_type.go` (IsLinked), `modelsdk/widgets/loader.go`, `mdl/executor/widget_engine.go` (`refuseLinkedDataSourceMapping`)", "insight": "Before mapping a widget property, check `isLinked` in widget.xml — a linked property is the platform's to fill, the ADR-0005 'author only what the model owns' rule wearing a widget hat. Three cheap measurements settle it faster than reasoning: grep the shipped template's ValueType for IsLinked, dump the property off Studio Pro-authored widgets in testdata/expr-checker (5 of 5 store linkedDs empty), and mx check the correct shape (0 errors WITHOUT it). Beware the inverted signal: writing the value does NOT clear CE0642, so a failing check after writing it looks like the value is missing rather than unwanted. Across all widget packages in testdata, linkedDs is the ONLY linked datasource among the 8 multi-datasource widgets — DROPDOWNFILTER is single-source from MDL's side, ComboBox and the 6 charts are genuinely multi-source", "ce": ["CE0642"]} {"area": "mdl/executor", "date": "2026-09-16", "symptom": "A chart series given BOTH a static and a dynamic datasource writes its static x/y attributes against the DYNAMIC source's entity — mxbuild reports CE1613 \"The selected attribute 'CH.Forecast.Region' no longer exists.\"", "cause": "buildObjectListItem pre-resolves every datasource the item configures and dropped each resolved entity into the one shared pageBuilder.entityContext, so the LAST one won. The per-property link was already in hand and ignored: ItemPropertyMapping.DataSource carries widget.xml's `dataSource=\"...\"` and GenerateDefJSON already emits it for every chart dependent", "file": "`mdl/executor/widget_engine.go` (`itemEntityContextFor`, `prebuiltEntities` in `buildObjectListItem`)", "insight": "The item twin of the widget-level per-datasource context (#1109). Look for the SECOND copy whenever a context fix lands at widget level — object-list items run the same pre-resolve/resolve shape with their own loop. The shipped chart defs already map staticDataSource AND dynamicDataSource with every dependent's link, so nothing needed mapping; the links simply were not read. Note the weak in-repo signals: `mxcli check` only warns (MDL-WIDGET10, the inactive set is hidden) and the describe output looks right, so the defect is visible only in the stored BSON or from mxbuild. Charts' static/dynamic sit INSIDE the `lines` object list, not at widget level — a recursive widget.xml scan makes them look like widget properties", "ce": ["CE1613"]} +{"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY MICROFLOW` re-enables concurrent execution on a microflow that disallowed it \u2014 the running app's concurrency protection removed \u2014 and drops the concurrency error message (all translations) and error microflow, plus `MarkAsUsed`. Every checker is green: **CE4899 fires only on disallow-without-a-message, never on allow**, so the one error that exists in this area is exactly the one the reset switches off", "cause": "`buildMicroflowFromStmt` built the rebuild struct with `AllowConcurrentExecution: true` and `MarkAsUsed: false` literals, and `microflowToGen` wrote `SetConcurrencyErrorMicroflowQualifiedName(\"\")` + a bare `genTexts.NewText()`. The backend already READ the two flags back (the #723 \u00a7A fix), so the round-trip test passed while the bug was live \u2014 the executor overwrote them before the backend ever saw them", "file": "`mdl/executor/cmd_microflows_build.go` (buildMicroflowFromStmt), `mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `sdk/microflows/microflows.go`", "fix": "Carry all four from the stored microflow, seeding the locals with the NEW-microflow defaults (true/false) so no separate preserve flag is needed. The error message reuses the existing `textFromGen`/`textToGen` pair, so translations survive; nil still emits the bare empty `Texts$Text` the writer always wrote", "insight": "**A passing round-trip test at one layer says nothing about the layer above it.** `TestMicroflowRoundTrip_ConcurrentExecutionFlags` had guarded these two flags since #723 and was green throughout, because the executor's rebuild struct overwrites them before calling the backend. When a property is reset, locate the LAST writer on the path, not the first one that looks responsible. **And check which way a reset goes**: #723's backend bug wrote the Go zero value (allow -> disallow) and hit CE4899 immediately; the executor's literal writes the opposite (disallow -> allow), and the same CE4899 that caught the first direction is structurally blind to the second. A checker that catches a property's loss in one direction is not coverage for that property. Two methodological traps in the test itself, both hit: `bytes.Equal` on two encodes of the same microflow ALWAYS differs (fresh random sub-element `$ID`s \u2014 the reason `canon` exists), and `canon.Equal` on a whole microflow always differs too, because `StableId` is a fresh GUID *value* per encode and `Equal` does not mask \u2014 only `Reconcile` may be asked that question. Compare the sub-element under test, or use Reconcile. Controls: hardcoding the executor literals back, emptying the writer's pair, and stubbing the reader each fail a different test with the reported symptom"} diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 3ced6780e2..15769f85a1 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -64,3 +64,5 @@ {"area":"mdl/catalog","date":"2026-09-13","symptom":"A user asks 'are there circular dependencies between my modules?', reads CATALOG.GRAPH_CYCLES, gets 0 rows, and concludes there are none — while CATALOG.GRAPH_MODULE_COUPLING lists the same module pair in both directions (mendixlabs/mxcli#1060)","cause":"GRAPH_CYCLES is SCCs of the ASSET graph. Modules A and B reference each other through DIFFERENT documents, which form no cycle among themselves, so the asset table is correctly empty for a genuinely circular module pair. Compounded by scope: the asset graph admits only graphRefKinds, while coupling counts every kind — 110 of 316 edges on a stock 11.14 app, and Administration -> Atlas_Core there is `layout`-only","file":"`mdl/catalog/builder_graph.go` (`buildModuleCycles`, `loadModuleEdges`, `graphRefKindsSQL`), `mdl/catalog/tables.go` (`graph_module_cycles_data`, `graph_analysis_scope`), `mdl/linter/starlark_graph.go` (`module_cycles()`)","insight":"**When a report says two tables disagree, check whether they answer different questions before checking whether one is broken — then make the difference queryable.** Tarjan was fine; the granularity and the edge filter were both undocumented and invisible from SQL. Two traps in the implementation: (1) the new module pass sat after `if len(edges) == 0 { return }` on the STRUCTURAL edge set, so a project whose only cross-module refs are navigational — the reported case exactly — got nothing; a test with only `layout` edges caught it. (2) A module-cycle table built on the structural subset would have shipped green and still answered 'none' for the reported pair, so the edge set has to match graph_module_coupling, the table it is read beside. Generate the scope view's IN list from graphRefKinds rather than restating it, or the documentation of the filter drifts from the filter","fix":"Add graph_module_cycles (module SCCs, all kinds, with a RefKinds column naming the edges inside the cycle) and graph_analysis_scope (per-kind edge counts + InAssetGraph), plus a module_cycles() Starlark builtin"} {"area": "mdl/ast", "date": "2026-09-15", "symptom": "Rewriting a microflow from its own `describe microflow` output moves its workflow actions: after `create or modify`, `open workflow` / `notify workflow` (and every other workflow action) sit ~800px further right, the first one on top of the end event; a second rewrite is stable. Five `log` statements round-trip identically", "cause": "`@position` (and `@caption`, `@color`, `@anchor`) was parsed and dropped for these statements: the visitor's `setStatementAnnotations` and the builder's `getStatementAnnotations` were hand-written type switches with no case for any of the eleven workflow statements or the three mapping statements (import/export mapping, transform json). The setter also had an EMPTY `case *ast.EnumSplitStmt:` — a no-op in Go, which does not fall through — and the getter lacked `SendRestRequestStmt`. With no annotation the builder auto-placed each action after the start event, which the stored start position had moved right", "file": "`mdl/ast/annotations.go` (new `SetStatementAnnotations`), `mdl/visitor/visitor_microflow_statements.go` (`setStatementAnnotations`), `mdl/executor/cmd_microflows_builder_annotations.go` (`getStatementAnnotations`)", "insight": "**This is #884's lesson, unapplied one level up.** #884 introduced the reflective `ast.StatementAnnotations` precisely because a type switch over the annotated statements silently skips the one added later — but only the validator used it, while the two switches that actually carry `@position` from source to BSON stayed hand-written and had already missed 15 types. When a class-level fix lands, grep for every other copy of the pattern it replaces. Both now delegate to reflection, so a new statement type with an `Annotations` field is covered on declaration; `TestEveryAnnotatedStatementIsReachable` pins the field shape. **Diagnose with the source, not the symptom list**: a go/parser scan of the AST structs against the switch case labels found all 15 gaps (and the empty case) in one run, where the report named two statements. Control: with both switches restored, `TestPositionAnnotationPlacesEveryActionStatement` fails 17 subtests at the builder default (100,100) while its `log` subtest passes. Measured on mx-test-projects/i956 (11.13): describe→rewrite→describe diffs every position before, identical after. Tests `mdl/executor/microflow_statement_position_test.go`, `mdl/ast/annotations_coverage_test.go`", "refs": [], "rules": []} {"area": "mdl/exprcheck", "date": "2026-09-16", "symptom": "`$out = $out + $r/Status` inside `LOOP $r IN $reqs` (Enumeration into a String) passes `mxcli check -p --references`, is written by `exec`, and fails the native build with **CE0117** at the Change variable activity. The same mistake on a PARAMETER is refused as E004, so the checker looks like it is skipped inside LOOP bodies (mendixlabs/mxcli#1100)", "cause": "The loop BODY was walked and checked all along \u2014 the control that proves it is `'status=' + $T/Status` on a parameter written one line INSIDE the loop, which was refused before the fix. Two holes in the variable scope, in series, produced the asymmetry: (a) `buildVarEntityScope` recorded CREATE, database RETRIEVE and parameters but never `LoopStmt.LoopVariable`, so `$r/Status` resolved to no attribute and inferred KindUnknown, which every rule tolerates by design; (b) `CheckAdapter` never set `Context.Scope` at all, so a DECLARE'd `$out String` was Unknown too \u2014 and E004 needs BOTH operands typed, so closing (a) alone still reported nothing on the reported script. (b) also meant `$out = $out + $Req/Status` with no loop in sight was equally silent; the report's own case A hides that by using a string literal on the left", "file": "`mdl/exprcheck/adapters/adapter_scope.go` (`buildFlowScope` replacing `buildVarEntityScope`, `recordRetrieve`/`recordListOperation`/`recordDeclare`, `kindScope`, `StatementErrorHandling`, `DataTypeKind`), `mdl/exprcheck/adapters/check.go` (`walkFlow` passes Scope; `checkListOperationCondition`; ON ERROR bodies walked), `mdl/exprcheck/slot_resolver.go` + `slot_to_context.go` (`ListOperation.Condition`), `mdl/executor/validate_microflow.go` (delegates `astKindToExprKind` and `stmtErrorHandling`)", "insight": "**Separate \"was the walk there\" from \"did the variable resolve\" before believing a skipped-construct report.** The title said LOOP bodies were not checked; one control \u2014 the same expression on a parameter, one line deeper \u2014 showed the walk was fine and the scope was not, which changed the fix from a walk to a resolver. **A silence can need two fixes to break**: typing the loop variable alone left the reported script still reporting nothing, because the rule needs both operands. Fix one, re-measure, and do not conclude the fix failed. **The same walk already existed, correct, next door**: `mdl/executor/validate_member_refs.go` typed loop variables from the list; the expression checker's walk did not \u2014 duplicate-resolver drift, which is why `stmtErrorHandling` and the DataTypeKind table are now single copies in adapters with the executor delegating. **Order is load-bearing and silent when wrong**: parameters must seed the scope BEFORE the body walk, or an association retrieve off a parameter (and every loop over its result) stays untyped \u2014 this was written the old way first and only a test caught it. **False-positive control**: exec-then-type-check over 591 mdl-examples scripts, 11 violations before and 11 after, same rules. It earned its keep \u2014 the first cut fired E009 on `set $At = find($Hay, $Needle)`, Mendix's STRING find, which the visitor still builds as a ListOperationStmt (the flow builder disambiguates it later, ledger #63). Requiring a KNOWN element entity before checking a FIND/FILTER predicate applies the same disambiguation. Controls: each of the four scope sources reverted in turn fails a distinct test with the reported symptom (empty violations). **Still open**: a bare attribute name in a FILTER predicate resolves to nothing, `retrieve \u2026 limit 1` is typed as a list like any other retrieve, and `LOOP $r IN $T/Mod.Assoc` cannot be typed because the visitor drops the association path (`ListVariable` is empty)"} +{"area": "mdl/catalog", "date": "2026-09-17", "symptom": "Every microflow behind a published REST endpoint reads as dead: `CATALOG.GRAPH_DEAD_ASSETS` lists it, `SHOW CALLERS OF` says \"(no callers found)\", `SHOW REFERENCES TO` and `impact` report nothing, and QUAL004 says \"is not called from anywhere. Remove if unused.\" On the reporter's model, 92 of 93 published operations name a microflow and all 92 were listed dead \u2014 15 percent of its dead-microflow list, pointed at the most exposed code in the app (mendixlabs/mxcli#1126)", "cause": "`buildPublishedRestServices` wrote `published_rest_operations_data.Microflow` and returned \u2014 it appended to no slice that `buildReferences` drains, and there was no RefKind for the edge. The binding was in the catalog; the edge was not. Fourth instance of one class (widget actions #773, scheduled events, project settings, this)", "file": "`mdl/catalog/builder.go` (`publishedRestRefs`), `mdl/catalog/builder_rest.go` (`publishedRestRef`, `publishedRestOpName`, collection in the operation loop), `mdl/catalog/builder_references.go` (`RefKindPublish`, `extractPublishedRestRefs`), `mdl/catalog/builder_graph.go` (`graphRefKinds`), `mdl/executor/cmd_search.go` (`callerRefKinds`), `.claude/lint-rules/orphaned_elements.star` (`MICROFLOW_ENTRY_KINDS`); tests `mdl/catalog/builder_rest_refs_test.go`, `mdl/catalog/lint_rule_vocabulary_test.go`, `mdl/executor/cmd_search_callers_test.go`", "insight": "**`GRAPH_DEAD_ASSETS` is kind-AGNOSTIC \u2014 the comment beside `schedule` in `graphRefKinds` says otherwise and is wrong.** The view is `NOT EXISTS (SELECT 1 FROM refs WHERE TargetName = \u2026)`; `git log -L` shows it has never filtered on RefKind. That false comment sent the issue's root-cause analysis down the wrong path, and it would have sent the fix there too: `graphRefKinds` matters for the ANALYSIS graph (communities/layers/cycles/centrality), not for the dead list. Measured with a one-kind insert: a `publish` row in neither `graphRefKinds` nor `callerRefKinds` still took the microflow from dead=1 to dead=0. **Check which consumers actually filter before assuming all four do**: `impact` and `SHOW REFERENCES TO` select every kind, so the refs row alone fixes them; only `SHOW CALLERS` and QUAL004 need a vocabulary edit. **The three vocabularies drift independently and nothing tests the union** \u2014 `settings` shipped in v0.22.0 into refs and into QUAL004 but NOT into `callerRefKinds`, so `show callers of ` was still blind two releases later; found only by auditing the lists while adding a fourth kind, and fixed here alongside. **`sync` looks like an entry point and is not**: it targets an ENTITY, so it belongs with `datasource`/`retrieve` in the excluded set \u2014 the test now pins it there, because the next person adding a kind will read the list, not the builder. **Carry the source's own id on the edge**: the operation's synthetic `opID` was already computed for `published_rest_operations_data`, so passing it as `SourceId` (the scheduled-event precedent passes \"\") makes 'who calls this microflow' one join from the endpoint's path and summary. **Controls**: stubbing the extractor to emit nothing reproduces \"reported dead\" verbatim, and dropping the empty-microflow guard fails the two-operation test \u2014 the suite is green against neither. Adjacent and NOT fixed: `business_events_data.PublishMicroflow`/`SubscribeMicroflow` emit no edge either, and `PublishedRestService.AuthenticationMicroflow` (and its OData sibling) is on gen but never read into the semantic model, so a REST auth handler is invisible to mxcli entirely", "refs": ["mendixlabs/mxcli#1126", "mendixlabs/mxcli#773"], "rules": ["QUAL004"]} +{"area": "mdl/catalog", "date": "2026-09-17", "symptom": "A microflow that runs only as an **entity event handler** is reported as unused from three directions at once: `show callers of Mod.ACT_Order_Validate` says `(no callers found)`, `CATALOG.GRAPH_DEAD_ASSETS` lists it, and `mxcli lint` emits `[QUAL004] ... is not called from anywhere.` with the suggestion **\"Remove if unused\"** \u2014 on a microflow that runs on every commit. Reported as 32 dead of 36 handlers across 24 entities", "cause": "`mdl/catalog` touched `Entity.EventHandlers` in exactly one place and threw the list away: `hasEventHandlers = 1` in `builder_modules.go`. No `refs` row was ever emitted, and no table held the handlers, so the reference graph had no ENTITY -> MICROFLOW edge for them. The `calculate` edge two lines below in `buildReferences` is the same shape and was already there, which is why the infrastructure looked complete", "file": "`mdl/catalog/builder_entity_events.go` (new), `mdl/catalog/builder_references.go` (`RefKindEvent` + `extractEventHandlerRefs`), `mdl/catalog/tables.go` (`entity_event_handlers_data` + view, `CatalogSchemaVersion` 11->12), `mdl/catalog/catalog.go` (`Tables()`), `mdl/catalog/builder.go` (field + build step), `mdl/catalog/builder_graph.go` (`graphRefKinds`), `mdl/executor/cmd_search.go` (`callerRefKinds`), `.claude/lint-rules/orphaned_elements.star`", "insight": "**The third consumer of a new RefKind is a schema version, not a list.** Beyond the three kind lists the scheduled-event fix named (`callerRefKinds`, `graphRefKinds`, the QUAL004 rule), a new edge needs `CatalogSchemaVersion` bumped: refs are only written by REFRESH CATALOG FULL and `NewFromFile` applies the schema with CREATE TABLE IF NOT EXISTS, so without the bump an existing `.mxcli/catalog.db` gains the empty table and keeps serving the pre-fix edge set \u2014 the wrong answer, from a cache, after the fix shipped. `migrateIfSchemaMismatch` drops and rebuilds on a mismatch (verified by hand-editing catalog_meta back to '11'). **A flag is a missing table wearing a value**: `HasEventHandlers` and `NavigationProfile.OfflineEntityCount` are the same defect, and the fix is the same pair \u2014 rows for what it does, an edge for whether it is reachable. Do not encode the detail in the kind: eight kinds (`before_commit`, `after_delete`, ...) would enter every consumer's list to say one thing, so the moment/event go in the table and the edge stays one `event`. **The control has to be the binary, not the test**: stubbing `extractEventHandlerRefs` to emit nothing and rebuilding reproduced `(no callers found)` + 2 dead microflows on the same project, which is what proves the assertion detects something. mendixlabs/mxcli#1127; repro `mdl-examples/bug-tests/catalog-1127-entity-event-handler-refs.mdl`", "refs": ["mendixlabs/mxcli#1127"]} diff --git a/.claude/skills/fix-issue/findings/sdk.jsonl b/.claude/skills/fix-issue/findings/sdk.jsonl index 8fd781469a..dd5df4cca5 100644 --- a/.claude/skills/fix-issue/findings/sdk.jsonl +++ b/.claude/skills/fix-issue/findings/sdk.jsonl @@ -42,3 +42,4 @@ {"area": "sdk/mpr", "date": "2026-09-06", "symptom": "`mx check` reported CE0066 \"Entity access is out of date\" at \"Domain model of module 'BusinessEvents'\" after `create or modify persistent entity BusinessEvents.PublishedBusinessEvent ( EventId: long )` over the real BusinessEvents 3.12.0 marketplace module. LEGACY ENGINE ONLY — the codec engine produced 0 errors from the same script. Caught by the integration gate (TestMxCheck_DoctypeScripts/13-business-events-examples.mdl/legacy), not by any unit test.", "cause": "ReconcileMemberAccesses in sdk/mpr/writer_security.go skipped any rule whose MemberAccesses list held only the storage marker (`if len(maArr) <= 1 { break }`), so it never topped one up. A rule with zero member entries on an entity that HAS members is precisely the out-of-date state CE0066 names, so the skip left behind the one thing the function exists to prevent. Nothing reached that state until `create or modify entity` started PRESERVING access rules instead of deleting them: the rewrite dropped all five attributes the Administrator rule covered, the prune emptied the list, and the new EventId then never got an entry. Fixed by narrowing the guard to `len(maArr) == 0` (no storage marker at all).", "file": "`sdk/mpr/writer_security.go` (ReconcileMemberAccesses, the MemberAccesses loop); tests `sdk/mpr/writer_security_reconcile_test.go`", "insight": "A fix that starts PRESERVING something reaches states no prior code could produce, so its blast radius is every consumer of that thing — here a reconcile function untouched for months. The engine split is the tell worth acting on: identical script, 0 errors on modelsdk and CE0066 on legacy, which localises the defect to the legacy path in one measurement and makes the codec engine the reference for what the document should contain (dumped both: 1 member entry vs 0). Also note where this was caught — only the integration gate exercises a real marketplace module, and only that module had a rule whose entire member set the script drops. The unit tests written for the entity fix were green throughout, and were right to be: the entity layer did exactly what it should. Keep the empty-list case as a named test on both sides, with a member-less entity as the control, since the old guard covered that case by accident and removing it must not turn every member-less entity into a write."} {"area": "sdk/mpr", "date": "2026-09-12", "symptom": "The legacy writer's image widgets disagree with Studio Pro. `serializeStaticImage` omits AlternativeText entirely; `serializeDynamicImage` writes one containing a `FallbackValue` string; both write BSON null for the unset Image / DefaultImage. mxbuild accepts all of it at 0 errors", "cause": "`Forms$ClientTemplate` has exactly three properties — Fallback (Texts$Text), Parameters, Template (generated/metamodel, and all three Studio Pro references). The dynamic image hand-rolled its own holder instead of calling `serializeClientTemplate`, and invented FallbackValue. AlternativeText is declared without omitempty on both image types and appears in 3/3 references, so omitting it is a drop, not an optional key", "file": "`sdk/mpr/writer_widgets_display.go` (serializeStaticImage, serializeDynamicImage, emptyAlternativeText)", "insight": "**A hand-rolled copy of a shared serializer is where the invented key lives.** The correct helper was four lines away and carried a comment naming this exact mistake; the copy still got it wrong, because nothing compares the two. Grep for a type's $Type string and check whether every construction site goes through one builder. **mxbuild is not a check for this class at all** — it tolerates unknown properties, while Studio Pro resolves every stored property against the type's property list and throws \"Sequence contains no matching element\" at MprProperty.cs. The available substitutes are generated/metamodel (the arbiter) and a real Studio Pro document from a marketplace module in the fixture", "refs": []} {"area": "sdk/mpr", "date": "2026-09-14", "symptom": "Legacy engine: a user task's on-created microflow (set in Studio Pro) reads back as empty — `describe workflow` omits it and the semantic `UserTask.OnCreated` is \"\" — while the modelsdk engine reads it", "cause": "`parseUserTask` did `raw[\"OnCreatedEvent\"].(string)`, but the stored value is a PART document: `{ $Type: Workflows$MicroflowBasedEvent, Microflow: \"Mod.MF\" }` or `{ $Type: Workflows$NoEvent }`. The type assertion never matched, silently", "file": "`sdk/mpr/parser_workflow.go` (`parseUserTask`)", "insight": "A `.(string)` assertion on a key whose metamodel type is a part/by-name-in-a-part fails silently and yields the zero value — indistinguishable from 'not set'. When a field reads empty on one engine only, check the stored shape with a Studio Pro reference document (ako/TestApp) before assuming the model lacks it. The test round-trips the writer's own document through `bson.Marshal`/`Unmarshal` so the parser sees real decoded types; control: restoring the string assertion fails it"} +{"area": "sdk/versions", "date": "2026-09-17", "symptom": "On a Mendix 10.24.25 project, `CREATE PAGE Mod.P (Params: { $X: Mod.E })` is refused with \"create page with parameters requires Mendix 11.0+ (project is 10.24.25)\", so no parameterised page can be authored from MDL on any 10.x project at all — and `SHOW PAGE Mod.P ($X = $obj)` has nothing to bind to. Teams fall back to a microflow data source that re-derives the object from $currentUser. mendixlabs/mxcli#1121", "cause": "The version registry's floor was never measured. `pages.page_parameters: 11.0.0`, `microflows.show_page_with_params: 11.0.0` and `pages.page_variables: 11.0.0` match the illustrative `show features` sample table in docs/11-proposals/PROPOSAL_version_aware_agent_support.md and nothing else. Measured against the Mendix Model SDK's own StructureVersionInfo records (mendixmodelsdk 4.115.0, src/gen/pages.js): Pages$PageParameter and Page.parameters are 9.4.0, PageSettings.parameterMappings 9.7.0, Pages$LocalVariable 10.17.0 with its DefaultValue 10.20.0. What IS 11.5.0 is PageParameter.IsRequired and PageParameter.DefaultValue — which pageParameterToGen emitted unconditionally, on every version.", "file": "`sdk/versions/mendix-9.yaml`, `sdk/versions/mendix-10.yaml` (floors); `mdl/backend/modelsdk/page_write.go` (pageParameterToGen, now takes *types.ProjectVersion); `mdl/executor/cmd_pages_create_v3.go` (gate kept, it is still right for 9.0-9.3). Tests `sdk/versions/page_parameter_floor_test.go`, `mdl/backend/modelsdk/page_parameter_version_test.go`, example `mdl-examples/bug-tests/1121-page-parameters-on-mendix-10.mdl`", "insight": "**A version floor in the registry is a measurement, and the mendixmodelsdk npm package is where to take it** — every class and property carries a StructureVersionInfo with `introduced`/`deleted`, so `npm pack mendixmodelsdk` and grep beats reasoning about release notes. Nothing in the repo distinguished a measured floor from a made-up one, which is how a number from a proposal's *sample output* became the thing that refused users' scripts; the floors now carry a `notes:` naming the source. **A wrong gate and a wrong writer hid each other**: the 11.0 gate was masking that the writer emits two 11.5-only keys, so lifting the gate alone would have traded an honest refusal for a page Studio Pro cannot open (mxbuild accepts unknown properties; Studio Pro throws InvalidOperationException at MprProperty.cs) — check what the gate was compensating for before removing one. The same wrongness also means the bug was **live on 11.0-11.4**, which passed the gate and got both keys written; that row is the regression control in the writer test. **Split the element from its properties**: the thing being gated was a 9.4 element with an 11.5 tail, and one floor cannot express that — the registry gates the element, the writer gates the tail.", "refs": ["#1121"]} diff --git a/.claude/skills/implement-mdl-feature.md b/.claude/skills/implement-mdl-feature.md index c1fef7e700..e82ebbb960 100644 --- a/.claude/skills/implement-mdl-feature.md +++ b/.claude/skills/implement-mdl-feature.md @@ -47,8 +47,9 @@ Implementing a new MDL feature requires changes across multiple layers: ┌───────────────┴───────────────┐ ▼ ▼ ┌─────────────────────────────┐ ┌─────────────────────────────┐ -│ 6a. PARSER (BSON → Go) │ │ 6b. WRITER (Go → BSON) │ -│ sdk/mpr/parser_*.go │ │ sdk/mpr/writer_*.go │ +│ 6a. READER (BSON → Go) │ │ 6b. WRITER (Go → BSON) │ +│ mdl/backend/modelsdk/ │ │ mdl/backend/modelsdk/ │ +│ *_read.go │ │ *_write.go │ │ for describe to work │ │ for create to work │ └─────────────────────────────┘ └─────────────────────────────┘ │ │ @@ -340,7 +341,7 @@ type HttpConfiguration struct { ## Part 6a: Parser (BSON → Go) -Add parsing logic in `sdk/mpr/parser_microflow.go`: +Add read logic in `mdl/backend/modelsdk/microflow_read_actions.go`: ```go // add case in parseActionActivity switch @@ -371,7 +372,7 @@ func parseRestCallAction(raw map[string]interface{}) *microflows.RestCallAction ## Part 6b: Writer (Go → BSON) -Add serialization logic in `sdk/mpr/writer_microflow.go`: +Add serialization logic in `mdl/backend/modelsdk/microflow_write.go`: ```go func serializeRestCallAction(action *microflows.RestCallAction) bson.D { @@ -624,8 +625,8 @@ Before considering the implementation complete: - [ ] AST types added (`mdl/ast/`) - [ ] Visitor implemented (`mdl/visitor/`) - [ ] SDK types added/updated (`sdk/microflows/` or `sdk/pages/`) -- [ ] BSON parser added (`sdk/mpr/parser_*.go`) -- [ ] BSON writer added (`sdk/mpr/writer_*.go`) +- [ ] BSON reader added (`mdl/backend/modelsdk/*_read.go`) +- [ ] BSON writer added (`mdl/backend/modelsdk/*_write.go`) - [ ] Executor builder added (`mdl/executor/cmd_*_builder.go`) - [ ] Executor show/describe added (`mdl/executor/cmd_*_show.go`) - [ ] Syntax check passes @@ -646,10 +647,11 @@ Before considering the implementation complete: | Visitor | `mdl/visitor/visitor_page.go` | Page parsing | | SDK | `sdk/microflows/microflows_actions.go` | Action Go types | | SDK | `sdk/pages/pages.go` | Widget Go types | -| Parser | `sdk/mpr/parser_microflow.go` | BSON → Go (microflows) | -| Parser | `sdk/mpr/parser_page.go` | BSON → Go (pages) | -| Writer | `sdk/mpr/writer_microflow.go` | Go → BSON (microflows) | -| Writer | `sdk/mpr/writer_widgets.go` | Go → BSON (widgets) | +| Reader | `mdl/backend/modelsdk/microflow_read_actions.go` | BSON → Go (microflows) | +| Reader | `mdl/backend/modelsdk/page.go` | BSON → Go (pages) | +| Writer | `mdl/backend/modelsdk/microflow_write.go` | Go → BSON (microflows) | +| Writer | `mdl/backend/modelsdk/widget_write.go` | Go → BSON (widgets) | +| Codec | `modelsdk/codec/encoder.go`, `decoder.go` | the layer underneath both | | Executor | `mdl/executor/cmd_microflows_builder.go` | AST → microflow BSON | | Executor | `mdl/executor/cmd_microflows_show.go` | Microflow → MDL | | Executor | `mdl/executor/cmd_pages_builder.go` | AST → page BSON | diff --git a/.claude/skills/maintain-wiki.md b/.claude/skills/maintain-wiki.md index f5887e8a3c..0ec580a9a8 100644 --- a/.claude/skills/maintain-wiki.md +++ b/.claude/skills/maintain-wiki.md @@ -16,7 +16,7 @@ Six page categories. Anything outside these belongs somewhere else. (e.g. association `ParentPointer`/`ChildPointer` inversion, storage names vs qualified names, version gating). 3. **Design rationale** — *why* the project is shaped this way (e.g. why MDL - is SQL-shaped, why the executor must not import `sdk/mpr` for writes, why + is SQL-shaped, why the executor must not reach past `ctx.Backend` for writes, why pure-Go SQLite). 4. **Project positioning** — how mxcli relates to its neighbours (TypeScript SDK, Mendix Studio Pro), what is intentionally not implemented. diff --git a/.claude/skills/maintain-wiki/pages.md b/.claude/skills/maintain-wiki/pages.md index b498096dc6..3bdfcff83d 100644 --- a/.claude/skills/maintain-wiki/pages.md +++ b/.claude/skills/maintain-wiki/pages.md @@ -10,7 +10,7 @@ | `models/storage-vs-qualified-names.md` | mental-model | BSON `$type` vs SDK qualified name | | `models/version-gating.md` | mental-model | feature registry, `min_version`, `checkFeature()` | | `rationale/mdl-as-sql.md` | rationale | why MDL is SQL-shaped, design principles (cites ADRs) | -| `rationale/backend-abstraction.md` | rationale | why the executor never imports `sdk/mpr` for writes (cites ADRs) | +| `rationale/backend-abstraction.md` | rationale | why the executor never reaches past `ctx.Backend` for writes (cites ADRs) | | `positioning/vs-typescript-sdk.md` | positioning | gap analysis, intentional differences | | `glossary.md` | glossary | Mendix ↔ mxcli ↔ BSON term bridge | | `bug-patterns/bson-numeric-width.md` | bug-pattern | int32/int64 mismatches (links #583, #585 findings) | diff --git a/.claude/skills/mendix/bootstrap-app/SKILL.md b/.claude/skills/mendix/bootstrap-app/SKILL.md index 97935b51e9..e6b69f65ae 100644 --- a/.claude/skills/mendix/bootstrap-app/SKILL.md +++ b/.claude/skills/mendix/bootstrap-app/SKILL.md @@ -1,6 +1,6 @@ --- name: bootstrap-app -description: "Provision a Mendix project in a repo that has none — interview, `mxcli new`, session hook, project brief, first commit and boot. Use when the repository is empty or has no .mpr yet, typically from the empty-repo seed prompt." +description: "Provision a Mendix project in a repo that has none — interview, `mxcli new`, session hook, project brief, first commit, boot and quality baseline. Use when the repository is empty or has no .mpr yet, typically from the empty-repo seed prompt." --- # Bootstrap a Mendix App in an Empty Repo @@ -20,7 +20,9 @@ folder and go straight to the work. Related skills: `run-local` (the warm dev loop this ends in), `mdl-entities` and `create-page` (building the model you propose at the end), -`migrate-design-prototype` (when a design was handed to you). +`migrate-design-prototype` (when a design was handed to you), `assess-quality` +(reading the `report` this takes a baseline with), `test-microflows` and `test-app` +(the two gates that prove behaviour rather than syntax). --- @@ -190,12 +192,27 @@ drop the `./` if it came pre-installed on `PATH`. clone, so committing the script is what makes the hook survive a reap. 8. **Boot and verify:** `./mxcli run --local -p .mpr` in the background, then confirm the app answers HTTP 200 at http://localhost:8080/ and report. -9. **(Optional) browser preview from a cloud session:** - `./mxcli run --hub https://hub.mxcli.org -p .mpr`, and report the preview - URL it prints. Needs `MXCLI_HUB_KEY` on the environment; without it, continue as a - normal local run. `--hub` ships in the **Linux** build only (a cloud session is a - Linux container, so it works there); on a native Windows/macOS mxcli it fails with - an explanatory message — continue as a normal local run. +9. **Take the quality baseline** — run the two gates that score the project, on the + blank app, before any of your own work is in it: + + ```bash + ./mxcli lint -p .mpr + ./mxcli report -p .mpr --format markdown + ``` + + Report the warning count and the six category scores, and put them in + `FINDINGS.md` with the date. This is the only moment the numbers mean "what the + template ships with" — afterwards every figure is yours plus the template's, and + there is nothing to subtract. A blank app is **not** expected to score zero + warnings; knowing which ones it starts with is what stops you chasing them later. + Read `.ai-context/skills/assess-quality/SKILL.md` before interpreting the report — + it covers what each category means and which findings are worth acting on. +10. **(Optional) browser preview from a cloud session:** + `./mxcli run --hub https://hub.mxcli.org -p .mpr`, and report the preview + URL it prints. Needs `MXCLI_HUB_KEY` on the environment; without it, continue as a + normal local run. `--hub` ships in the **Linux** build only (a cloud session is a + Linux container, so it works there); on a native Windows/macOS mxcli it fails with + an explanatory message — continue as a normal local run. --- @@ -294,6 +311,34 @@ see `migrate-design-prototype`. ./mxcli exec change.mdl -p .mpr # edit the model; the loop hot-applies ``` +### The gates — the same list the project's CLAUDE.md publishes + +`mxcli init` wrote these into the project's `CLAUDE.md`, so every later session has +them in context. They are the **definition of done**, not a menu: run them in order, +stop at the first that fails, and say what each one reported. + +```bash +./mxcli check change.mdl -p .mpr --references # syntax + references (~2s) +./mxcli exec change.mdl -p .mpr # apply +./mxcli lint -p .mpr # rules (~3s) +./mxcli report -p .mpr # scored quality report +./mxcli docker check -p .mpr # mxbuild, the slow one (~25s) +./mxcli test tests/ -p .mpr --local # microflow tests (~30s cold, ~2s warm) +./mxcli run --local --watch -p .mpr # the app, hot-reloading +``` + +Two of them are easy to mistake for optional and are not: + +- **`report` is the quality report**, and its six category scores are what the + baseline in step 9 exists to be compared against. A score that fell is a finding, + not a detail. `assess-quality` covers how to read it. +- **`test` needs a suite to run.** Write the first one with the **first microflow you + build** — not "later", because later is after the code is written and the expected + values have stopped being obvious. One `tests/.test.mdl` per slice is the + shape that keeps up; `test-microflows` has the annotations, and `--local` needs no + Docker daemon. For pages and rendering, `test-app` drives a real browser: a page can + serialize correctly, pass `check`, build clean and still render wrong. + Keep the plan current as you go — it is the only record of scope that outlives the conversation: diff --git a/.claude/skills/mendix/debug-bson/SKILL.md b/.claude/skills/mendix/debug-bson/SKILL.md index 81ce8f2522..c1b0d21a97 100644 --- a/.claude/skills/mendix/debug-bson/SKILL.md +++ b/.claude/skills/mendix/debug-bson/SKILL.md @@ -45,14 +45,14 @@ Symptoms that indicate BSON serialization issues: ### Step 3: Dump Both BSON Structures -Use the `mxcli dump-bson` command to extract and compare: +Use the `mxcli bson dump` command to extract and compare: ```bash # Dump the SDK-generated object (the broken one) -mxcli dump-bson -p app.mpr -o "PgTest.BrokenPage" > broken.json +mxcli bson dump -p app.mpr -o "PgTest.BrokenPage" > broken.json # Dump the Studio Pro-generated object (the fixed one) -mxcli dump-bson -p app.mpr -o "PgTest.FixedPage" > fixed.json +mxcli bson dump -p app.mpr -o "PgTest.FixedPage" > fixed.json # Compare the two diff broken.json fixed.json @@ -60,7 +60,7 @@ diff broken.json fixed.json Or use the `--compare` flag: ```bash -mxcli dump-bson -p app.mpr --compare "PgTest.BrokenPage" "PgTest.FixedPage" +mxcli bson dump -p app.mpr --compare "PgTest.BrokenPage,PgTest.FixedPage" ``` ### Step 4: Identify Differences @@ -77,10 +77,14 @@ Look for differences in: ### Step 5: Fix the Serialization Code -The serialization code lives in `sdk/mpr/writer_*.go` files: -- `writer_widgets.go` - Page widget serialization -- `writer_microflows.go` - Microflow activity serialization -- `writer_entities.go` - Entity/attribute serialization +The serialization code lives in `mdl/backend/modelsdk/*_write.go`: +- `widget_write.go` - Page widget serialization +- `page_write.go` - Page document (header, parameters, layout call) +- `microflow_write.go` - Microflow activity serialization +- `domainmodel_write.go` - Entity/attribute serialization + +The encoder and decoder underneath them are `modelsdk/codec/encoder.go` and +`modelsdk/codec/decoder.go`. ## Part 2: Common BSON Patterns @@ -185,52 +189,40 @@ func serializeClientTemplate(ct *pages.ClientTemplate) bson.D { ## Part 4: Debugging Tools -### Go Program for Raw BSON Dump - -Create a temporary Go program to inspect raw BSON: +### Dumping Raw BSON -```go -package main +`mxcli bson dump` reads the raw unit and prints it as JSON — no Go program needed: -import ( - "encoding/json" - "fmt" - "os" +```bash +# what is in there +mxcli bson dump -p app.mpr --type page --list - "github.com/mendix/modelsdk-go/sdk/mpr" -) +# one object +mxcli bson dump -p app.mpr --type page --object "MyModule.MyPage" -func main() { - reader, _ := mpr.NewReader(os.Args[1]) - defer reader.Close() +# the comparison that actually finds the bug: Studio Pro's vs mxcli's +mxcli bson dump -p app.mpr --type page --compare "MyModule.Broken,MyModule.Working" - docs, _ := reader.GetDocuments() - for _, doc := range docs { - if doc.Name == os.Args[2] { // Target object name - pretty, _ := json.MarshalIndent(doc.RawBSON, "", " ") - fmt.Println(string(pretty)) - } - } -} +# a byte-exact baseline to diff against later +mxcli bson dump -p app.mpr --type page --object "MyModule.MyPage" --format bson > baseline.mxunit ``` -### Using mxcli dump-bson - -```bash +`--type` also takes `microflow`, `nanoflow`, `enumeration`, `snippet`, `layout` +and `constant`. # list all pages in project -mxcli dump-bson -p app.mpr --type page --list +mxcli bson dump -p app.mpr --type page --list # list all microflows -mxcli dump-bson -p app.mpr --type microflow --list +mxcli bson dump -p app.mpr --type microflow --list # Dump specific page as json -mxcli dump-bson -p app.mpr --type page --object "PgTest.MyPage" +mxcli bson dump -p app.mpr --type page --object "PgTest.MyPage" # Save dump to file for comparison -mxcli dump-bson -p app.mpr --type page --object "PgTest.MyPage" > mypage.json +mxcli bson dump -p app.mpr --type page --object "PgTest.MyPage" > mypage.json # Compare two objects (outputs both as json) -mxcli dump-bson -p app.mpr --type page --compare "PgTest.Broken,PgTest.Fixed" +mxcli bson dump -p app.mpr --type page --compare "PgTest.Broken,PgTest.Fixed" # Supported types: page, microflow, nanoflow, enumeration, snippet, layout ``` @@ -282,7 +274,7 @@ When creating nested `WidgetObject` instances (e.g., DataGrid2 columns), creatin ```bash # Compare mxcli-generated vs Studio Pro-generated -mxcli dump-bson -p app.mpr --compare "PgTest.MDLPage,PgTest.StudioProPage" +mxcli bson dump -p app.mpr --compare "PgTest.MDLPage,PgTest.StudioProPage" # Look for property count differences: # ~ properties: array length differs (first: 5, second: 22) @@ -302,8 +294,8 @@ When building columns, iterate through ALL `PropertyTypes` in the template's `Ob 1. Count properties in both versions: ```bash - mxcli dump-bson -p app.mpr --type page --object "PgTest.BrokenPage" | grep "WidgetProperty" | wc -l - mxcli dump-bson -p app.mpr --type page --object "PgTest.FixedPage" | grep "WidgetProperty" | wc -l + mxcli bson dump -p app.mpr --type page --object "PgTest.BrokenPage" | grep "WidgetProperty" | wc -l + mxcli bson dump -p app.mpr --type page --object "PgTest.FixedPage" | grep "WidgetProperty" | wc -l ``` 2. Check the template for required properties: @@ -570,7 +562,7 @@ This applies to any executor function that reads column headers, button captions ## Related Documentation - [BSON Mapping Specification](../../docs/05-mdl-specification/10-bson-mapping.md) -- [Page Widget Serialization](../../sdk/mpr/writer_widgets.go) +- [Page Widget Serialization](../../mdl/backend/modelsdk/widget_write.go) - [Create Page Skill](../create-page/SKILL.md) - [Widget Templates README](../../sdk/widgets/templates/README.md) @@ -585,8 +577,8 @@ mxcli -p app.mpr -c "describe page PgTest.BrokenPage" # 2. create fixed version in Studio Pro, save project # 3. Dump both objects to json files -mxcli dump-bson -p app.mpr --type page --object "PgTest.BrokenPage" > broken.json -mxcli dump-bson -p app.mpr --type page --object "PgTest.FixedPage" > fixed.json +mxcli bson dump -p app.mpr --type page --object "PgTest.BrokenPage" > broken.json +mxcli bson dump -p app.mpr --type page --object "PgTest.FixedPage" > fixed.json # 4. Compare the json files diff broken.json fixed.json @@ -603,7 +595,9 @@ go build ./... && mxcli exec test.mdl -p app.mpr | File | Purpose | |------|---------| -| `sdk/mpr/writer_widgets.go` | Page widget BSON serialization | -| `sdk/mpr/writer_microflows.go` | Microflow BSON serialization | -| `sdk/mpr/writer_entities.go` | Entity BSON serialization | +| `mdl/backend/modelsdk/widget_write.go` | Page widget BSON serialization | +| `mdl/backend/modelsdk/microflow_write.go` | Microflow BSON serialization | +| `mdl/backend/modelsdk/domainmodel_write.go` | Entity BSON serialization | +| `modelsdk/codec/encoder.go` | Document → BSON | +| `modelsdk/codec/decoder.go` | BSON → document | | `docs/05-mdl-specification/10-bson-mapping.md` | BSON format documentation | diff --git a/.claude/skills/mendix/run-local/SKILL.md b/.claude/skills/mendix/run-local/SKILL.md index 91cfba3d28..8ffc53a8c6 100644 --- a/.claude/skills/mendix/run-local/SKILL.md +++ b/.claude/skills/mendix/run-local/SKILL.md @@ -82,8 +82,10 @@ association catalog only at startup; behavioural changes are hot-reloaded. The Mendix CDN publishes **Linux archives only** (the URL varies by architecture, not by OS), so a cached download on a Mac is a Linux `aarch64` ELF — the arch matches, which is why it looks fine until exec. -- `--mxbuild-path` overrides both, and is now honoured by the local loop (it used - to be documented and ignored — #916). +- `--mxbuild-path` overrides both. It is honoured by the local loop (#916) *and* + accepted by `run --local` (#1125) — between those two fixes the skill said the + first and the command rejected the flag, so the advertised workaround did not + exist on the platform that needed it. If nothing runnable is found, the command says so up front instead of failing with `fork/exec …: exec format error`: diff --git a/.claude/skills/mendix/validation-microflows/SKILL.md b/.claude/skills/mendix/validation-microflows/SKILL.md index 5c6e7eab93..5ad9749ca3 100644 --- a/.claude/skills/mendix/validation-microflows/SKILL.md +++ b/.claude/skills/mendix/validation-microflows/SKILL.md @@ -287,5 +287,5 @@ This feature is implemented in: - `mdl/visitor/visitor_microflow_statements.go` - ANTLR listener to build AST - `mdl/executor/cmd_microflows_builder.go` - Flow builder with variable validation - `mdl/executor/cmd_microflows_show.go` - DESCRIBE formatter for MDL output -- `sdk/mpr/writer_microflow.go` - BSON serialization for ValidationFeedbackAction +- `mdl/backend/modelsdk/microflow_write.go` - BSON serialization for ValidationFeedbackAction - `sdk/microflows/microflows_actions.go` - ValidationFeedbackAction struct diff --git a/.claude/skills/mendix/write-microflows/reference/pitfalls.md b/.claude/skills/mendix/write-microflows/reference/pitfalls.md index ad456e6775..fe7046d49a 100644 --- a/.claude/skills/mendix/write-microflows/reference/pitfalls.md +++ b/.claude/skills/mendix/write-microflows/reference/pitfalls.md @@ -538,3 +538,66 @@ It is a **security** setting and it only ever narrows, so the rules mirror no such property. Writing it there is **MDL059**, not a silent no-op — the same rule that catches `@applyentityacces` and any other annotation the document does not read. The message names what that document does accept. + +## Concurrency settings are preserved, not authorable + +Studio Pro's **"Disallow concurrent execution"**, its error message and error +microflow, and **"Mark as used"** have no MDL syntax. All four now survive a +`create or modify microflow`; before, the rebuild wrote its own values over +every one of them. + +The concurrency one is worth knowing about even though it is fixed, because of +which way it failed. The rebuild hardcoded *allow*, so a microflow that +**disallowed** concurrent execution came back allowing it — the running app's +concurrency protection silently removed. **CE4899 only fires on +disallow-without-a-message**, never on allow, so the one check that exists in +this area could not see it, and the error message went the same way, +translations included. + +There is nothing to write in a script. What matters is the same rule as below: +use `create or modify` to edit such a microflow, never `drop` + `create`. + +## Export level is preserved, not authorable + +A microflow carries an **export level** — Studio Pro's `Hidden` or `API` — which +decides whether it is part of the module's public surface when the module is +exported as a package. Like the URL below, MDL cannot write it, and like the URL +it now **survives a `create or modify microflow`**; before, every rewrite pinned +it to `Hidden`, quietly removing the microflow from a protected module's API. + +`Hidden` is the normal value by a wide margin — measured across Business Events +3.12.0 and External Database Connector 6.2.3/6.3.0, every document of every type +stores it — so `describe microflow` mentions the export level **only when it is +not `Hidden`**, as a `-- Export level:` comment. The copy caveat below applies to +it identically. + +## The deep-link URL is preserved, not authorable + +A microflow can carry a **URL** (Mendix 10.6+) — Studio Pro's "URL" field, e.g. +`item/{Key}` — which makes it reachable as a deep link. MDL has **no syntax for +it**, so there is no annotation to write and nothing to check. + +What matters is that it **survives**: a `create or modify microflow` that +rewrites the body keeps the stored URL and its search parameters. It did not +before #1120, and this one was harder to notice than the flags above, because a +microflow *without* a URL is a valid microflow — `mxcli check`, `mx check` and +mxbuild all reported success, and the deep link was simply gone the next time +someone opened Studio Pro. + +**A parameter in the URL path may not also be a search parameter.** mxbuild +rejects that combination with **CE5612** ("The Microflow parameter … cannot be +used as a URL parameter if it is already a URL search parameter"). Path +parameters and query parameters are disjoint sets. mxcli cannot author either, +so this only matters when reading a describe comment or reasoning about a +project — but it is the rule that decides whether a stored pair is valid. + +Two consequences for scripts: + +- **`describe microflow` emits it as a `-- URL:` comment**, not as executable + MDL, because there is nothing to execute. That comment is a warning, not + decoration: a **describe → rename → exec copy has nothing to preserve from**, + so the new microflow has no URL. Set it in Studio Pro after copying. +- **`drop microflow` followed by `create microflow` loses it** for the same + reason. Use `create or modify` to edit a microflow that has a deep link — or a + non-default export level or any concurrency setting, which the drop path loses + the same way. diff --git a/.claude/skills/version-awareness.md b/.claude/skills/version-awareness.md index 537282a878..ba45f7de6b 100644 --- a/.claude/skills/version-awareness.md +++ b/.claude/skills/version-awareness.md @@ -27,7 +27,6 @@ Common version gates: | Feature | Requires | Workaround for older versions | |---------|----------|-------------------------------| | VIEW ENTITY | 10.18+ | Regular entity with microflow data source | -| Page parameters | 11.0+ | Pass data via non-persistent entity | | REST query params | 11.0+ | Build query string manually in microflow | | DB runtime connection | 11.0+ | Hardcode connection in Database Connector config | | Design properties v3 | 11.0+ | Use Atlas v2 design properties | @@ -50,6 +49,6 @@ show features added since 10.24; -- what's new if upgrading from 10.24 Before writing any MDL for a connected project: 1. Run `show status` to confirm the Mendix version -2. If using view entities, page parameters, REST clients, or database queries — run `show features` to verify availability +2. If using view entities, REST clients, or database queries — run `show features` to verify availability 3. If a feature is unavailable, use the workaround pattern 4. Run `mxcli check script.mdl -p app.mpr --references` to validate before execution diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f8ce381ad..cb9abb0b68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Every microflow behind a published REST endpoint read as dead** (mendixlabs/mxcli#1126). The catalog recorded each operation's microflow in `published_rest_operations_data.Microflow` and emitted no `refs` edge to it, so a microflow whose only caller is an endpoint had zero inbound references: `CATALOG.GRAPH_DEAD_ASSETS` listed it, `SHOW CALLERS OF` answered "(no callers found)", `SHOW REFERENCES TO` and `impact` reported nothing, and lint rule **QUAL004** said "is not called from anywhere. Remove if unused." On the reported model 92 of 93 operations name a microflow and all 92 were listed dead — 15 percent of its dead-microflow list, aimed at the most exposed code in the app. + + A published REST operation is an entry point of the same shape as a scheduled event — the platform invokes it, so nothing in the model calls it — and now emits a **`publish`** edge from the operation to the microflow it runs. The source is the operation, not the service, so `show references to` names the one endpoint rather than the service holding thirty of them, and the edge carries the operation's catalog id, so its path and summary are one join away. + + Two corrections to the report's diagnosis, both measured. `GRAPH_DEAD_ASSETS` is **kind-agnostic** — it asks only whether any `refs` row targets the name — so the edge alone clears it; the comment beside `schedule` in `graphRefKinds` claiming otherwise was never true and is fixed. `impact` and `SHOW REFERENCES TO` do not filter by kind either. `publish` is still added to `graphRefKinds`, for the **analysis** graph: without it an API handler is an unreachable root in communities, layers, cycles and centrality. + + Found while auditing the three vocabularies: **`settings` was missing from `show callers`**. The project-settings edge shipped in v0.22.0 into `refs` and into QUAL004 but not into `callerRefKinds`, so `show callers of ` was still blind to it — fixed here too. `sync` is deliberately *not* a caller kind: it names an entity an offline profile downloads, which is a use of a type, and the test now pins it in the excluded set beside `datasource`. + - **The expression type checker was silent on a LOOP variable, and on every locally declared variable** (mendixlabs/mxcli#1100). `$out = $out + $r/Status` inside `LOOP $r IN $reqs` — an Enumeration concatenated into a String — passed `check --references`, was written by `exec`, and failed the build with **CE0117** at the Change variable activity, while the same mistake on a parameter was refused as **E004**. The report reads as "the checker is skipped inside a `LOOP` body". It is not, and the control says so: `'status=' + $T/Status` on a *parameter*, written one line **inside** the loop, was refused before this change. The body was always walked; the **variable scope** had two holes, and both had to close before the reported script reported anything. `LOOP $r IN $reqs` never recorded `$r`, so `$r/Status` resolved to no attribute and inferred Unknown — which every rule tolerates by design. And `Context.Scope` was never set at all, so a `DECLARE $out String` was Unknown too; E004 needs **both** operands typed, so typing the iterator alone still reported nothing. That second hole is why `$out = $out + $Req/Status` was equally silent with no loop in sight — the report's case A hides it by putting a string literal on the left. diff --git a/cmd/mxcli/cmd_bson_dump.go b/cmd/mxcli/cmd_bson_dump.go index fa15fa8bc9..d115aca8c5 100644 --- a/cmd/mxcli/cmd_bson_dump.go +++ b/cmd/mxcli/cmd_bson_dump.go @@ -40,7 +40,7 @@ Examples: mxcli bson dump -p app.mpr --type page --object "PgTest.MyPage" # Compare two objects (outputs both as JSON for diff) - mxcli bson dump -p app.mpr --type page --compare "PgTest.Broken" "PgTest.Fixed" + mxcli bson dump -p app.mpr --type page --compare "PgTest.Broken,PgTest.Fixed" # Save dump to file mxcli bson dump -p app.mpr --type page --object "PgTest.MyPage" > mypage.json diff --git a/cmd/mxcli/cmd_new.go b/cmd/mxcli/cmd_new.go index 8558f514d2..aded03f93e 100644 --- a/cmd/mxcli/cmd_new.go +++ b/cmd/mxcli/cmd_new.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "github.com/mendixlabs/mxcli/cmd/mxcli/docker" "github.com/mendixlabs/mxcli/cmd/mxcli/theme" @@ -85,6 +86,25 @@ Examples: os.Exit(1) } + // A hand-typed version may name no CDN artifact: Mendix 9 and 10 publish a + // build number the release notes never mention, so the release called + // "10.24.25" is mxbuild-10.24.25.122571.tar.gz and every probe of the + // three-part name 404s. Resolve before anything else, because the + // resolved string is what the rest of the command must use — the + // postcondition below compares the created project's stamp against it, + // and mx stamps the four-part version. + if resolved, rerr := docker.ResolveCDNVersion(mendixVersion, runtime.GOARCH); rerr != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", rerr) + if avail := docker.CDNReleasesFor(majorMinorOf(mendixVersion), runtime.GOARCH); len(avail) > 0 { + fmt.Fprintf(os.Stderr, " Published for %s: %s\n", + majorMinorOf(mendixVersion), strings.Join(avail[:minInt(len(avail), 8)], ", ")) + } + os.Exit(1) + } else if resolved != mendixVersion { + fmt.Printf("Resolved Mendix %s to %s\n", mendixVersion, resolved) + mendixVersion = resolved + } + // Step 1: Resolve mx binary. // On Windows and macOS, Studio Pro ships a native mx binary — prefer it. // CDN downloads contain Linux ELF binaries that cannot run on those platforms. diff --git a/cmd/mxcli/cmd_run.go b/cmd/mxcli/cmd_run.go index 3465b10673..a8c0f4bb99 100644 --- a/cmd/mxcli/cmd_run.go +++ b/cmd/mxcli/cmd_run.go @@ -146,6 +146,7 @@ Examples: appPort, _ := cmd.Flags().GetInt("app-port") adminPort, _ := cmd.Flags().GetInt("admin-port") servePort, _ := cmd.Flags().GetInt("serve-port") + mxbuildPath, _ := cmd.Flags().GetString("mxbuild-path") dbHost, _ := cmd.Flags().GetString("db-host") dbName, _ := cmd.Flags().GetString("db-name") dbUser, _ := cmd.Flags().GetString("db-user") @@ -200,6 +201,7 @@ Examples: AppPort: appPort, AdminPort: adminPort, ServePort: servePort, + MxBuildPath: mxbuildPath, Watch: watch, EnsureDB: ensureDB, SetupOnly: setupOnly, @@ -310,6 +312,7 @@ func init() { runCmd.Flags().Int("app-port", 0, "HTTP port for the app (default 8080)") runCmd.Flags().Int("admin-port", 0, "M2EE admin API port (default 8090)") runCmd.Flags().Int("serve-port", 0, "mxbuild --serve port (default 6543)") + runCmd.Flags().String("mxbuild-path", "", "Path to the mxbuild to build with, overriding resolution (Studio Pro's bundled mxbuild on macOS/Windows, the cached CDN download on Linux)") runCmd.Flags().String("db-host", "", "Database host:port (IPv6: [::1]:5432; default 127.0.0.1:5432)") runCmd.Flags().String("db-name", "", "Database name (default derived from the project name)") runCmd.Flags().String("db-user", "", "Database user (default mendix)") diff --git a/cmd/mxcli/cmd_run_mxbuildpath_test.go b/cmd/mxcli/cmd_run_mxbuildpath_test.go new file mode 100644 index 0000000000..d108d728e3 --- /dev/null +++ b/cmd/mxcli/cmd_run_mxbuildpath_test.go @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// `--mxbuild-path` was documented as the override for the local loop by the +// shipped run-local skill, by runlocal.go's own comment, and by two error +// messages that tell the user to pass it — while `run` never registered the +// flag, so it came back "unknown flag". On macOS that was the only advertised +// way out of a platform mismatch. (issue #1125) + +// TestRunCommandHasMxBuildPathFlag pins the flag's existence. The plumbing behind +// it (LocalRunOptions.MxBuildPath -> ResolveMxBuildForLocal) already worked; only +// the registration was missing, so nothing else could reveal the gap. +func TestRunCommandHasMxBuildPathFlag(t *testing.T) { + f := runCmd.Flags().Lookup("mxbuild-path") + if f == nil { + t.Fatal("run has no --mxbuild-path flag, but the run-local skill and two error messages tell users to pass it") + } + if f.Usage == "" { + t.Error("the flag needs help text; it is what users are pointed at when resolution fails") + } +} + +// TestRunMxBuildPathFlagIsAccepted is the reporter's command line, resolved +// through the real command tree so that `-p` (persistent, on root) is in scope — +// parsing alone is what failed, before any project was opened. +func TestRunMxBuildPathFlagIsAccepted(t *testing.T) { + cmd, args, err := rootCmd.Find([]string{"run", "--local", "--mxbuild-path", "/x", "-p", "app.mpr"}) + if err != nil { + t.Fatalf("finding run: %v", err) + } + if err := cmd.ParseFlags(args); err != nil { + t.Fatalf("mxcli run --local --mxbuild-path /x -p app.mpr: %v", err) + } + if got, _ := cmd.Flags().GetString("mxbuild-path"); got != "/x" { + t.Errorf("flag parsed to %q, want /x", got) + } +} + +// TestErrorGuidanceNamesAFlagThatExists is the general form of the defect: an +// error telling the user to pass an option the command does not have is worse +// than no guidance, because it reads as the user's mistake. Every --flag any +// mxbuild-resolution message recommends must be registered on `run`. +func TestErrorGuidanceNamesAFlagThatExists(t *testing.T) { + for _, src := range []string{ + filepath.Join("docker", "mxbuild_platform.go"), + filepath.Join("docker", "detect.go"), + filepath.Join("docker", "mxserve.go"), + } { + b, err := os.ReadFile(src) + if err != nil { + t.Fatalf("reading %s: %v", src, err) + } + if !strings.Contains(string(b), "--mxbuild-path") { + continue + } + if runCmd.Flags().Lookup("mxbuild-path") == nil { + t.Errorf("%s tells users to pass --mxbuild-path, which `run` does not accept", src) + } + } +} diff --git a/cmd/mxcli/docker/localapp.go b/cmd/mxcli/docker/localapp.go index 058bee2d54..897ef03173 100644 --- a/cmd/mxcli/docker/localapp.go +++ b/cmd/mxcli/docker/localapp.go @@ -22,6 +22,9 @@ type LocalAppOptions struct { ServePort int // AdminPass is the M2EE admin password (defaults to the local-run password). AdminPass string + // MxBuildPath overrides mxbuild resolution (optional), as --mxbuild-path does + // for `run --local`. Empty means resolve for this host. + MxBuildPath string // DB is the database to connect to; empty fields take the run --local // defaults (PostgreSQL at 127.0.0.1:5432, user/password mendix, database // name derived from the project file name). @@ -163,8 +166,15 @@ func StartLocalApp(opts LocalAppOptions) (*LocalApp, error) { version := reader.ProjectVersion().ProductVersion reader.Disconnect() - // 2. Cache mxbuild + runtime (no-ops when already present). - if _, err := DownloadMxBuild(version, w); err != nil { + // 2. Resolve mxbuild + cache the runtime (no-ops when already present). + // + // Resolution, not a bare DownloadMxBuild: the CDN publishes Linux archives + // only, so on macOS and Windows downloading caches a binary this host cannot + // execute. `run --local` was moved off DownloadMxBuild for that reason in + // #916; this second caller was left behind, which is why `test --local` still + // died on a Mac with Studio Pro installed (#1122). + mxbuildPath, err := ResolveMxBuildForLocal(opts.MxBuildPath, version, w) + if err != nil { return nil, fmt.Errorf("setting up mxbuild: %w", err) } installPath, err := resolveRuntimeInstall(version, w) @@ -197,7 +207,7 @@ func StartLocalApp(opts LocalAppOptions) (*LocalApp, error) { if !opts.SkipBuild { fmt.Fprintln(w, "Building project (mxbuild --serve)...") serveJavaMajor, _ := ProjectJavaMajor(opts.ProjectPath) - serve, err := StartServe(ServeOptions{Version: version, JavaMajor: serveJavaMajor, Host: "127.0.0.1", Port: opts.ServePort}) + serve, err := StartServe(serveOptionsFor(mxbuildPath, version, serveJavaMajor, opts.ServePort)) if err != nil { return nil, fmt.Errorf("starting mxbuild serve: %w", err) } diff --git a/cmd/mxcli/docker/mxserve.go b/cmd/mxcli/docker/mxserve.go index a48d69a2cf..c2ff88946d 100644 --- a/cmd/mxcli/docker/mxserve.go +++ b/cmd/mxcli/docker/mxserve.go @@ -26,10 +26,16 @@ import ( // ServeOptions configures StartServe. type ServeOptions struct { - // MxBuildPath is the mxbuild binary. When empty it is resolved from Version - // (CachedMxBuildPath) or the newest cached mxbuild (AnyCachedMxBuildPath). + // MxBuildPath is the mxbuild binary, and is what every local caller sets — + // resolved once, for this host, by ResolveMxBuildForLocal. + // + // When empty, resolution falls back to the cache: the entry for Version if + // there is one, and the newest cached entry only when no Version is given. It + // will NOT substitute a different version for a named one; see + // resolveServeMxBuild. MxBuildPath string - // Version is the Mendix version used to resolve mxbuild when MxBuildPath is empty. + // Version is the Mendix version used to resolve mxbuild when MxBuildPath is + // empty, and to refuse a cache entry that cannot build this project. Version string // JavaHome is the JDK home. When empty it is resolved for JavaMajor. JavaHome string @@ -186,19 +192,73 @@ func verifyMxBuildCache(mxbuildPath string) error { return nil } +// serveOptionsFor builds the ServeOptions for the local loop's build server. +// +// It exists so that the two local entry points — `run --local` (RunLocal) and +// `test --local` (StartLocalApp) — cannot construct these options differently. +// Each resolves the mxbuild this host can execute (ResolveMxBuildForLocal) and +// then has to hand it on; both used to omit it, so StartServe re-resolved from +// the cache and could start a DIFFERENT mxbuild than the one the caller had just +// chosen. On macOS that was the normal case, not an edge one: the caller picked +// Studio Pro's binary and the serve process got whatever the cache held, which +// is either a Linux download or another version entirely (#1122). +// +// MxBuildPath is the load-bearing field. The rest is plumbing. +func serveOptionsFor(mxbuildPath, version string, javaMajor, servePort int) ServeOptions { + return ServeOptions{ + MxBuildPath: mxbuildPath, + Version: version, + JavaMajor: javaMajor, + Host: "127.0.0.1", + Port: servePort, + } +} + +// resolveServeMxBuild decides which mxbuild `--serve` runs. +// +// A caller-supplied path wins, and is the path every local caller takes. When +// there is none, an exactly-matching cache entry is used — and, crucially, NOT a +// near miss: substituting another version here is how a project pinned to 11.12.2 +// came to be built by 11.14.0, a mismatch mxbuild itself only reports minutes +// later, after a cold model load. The unversioned fallback survives because a +// caller that names no version has nothing to be mismatched against. +func resolveServeMxBuild(opts ServeOptions) string { + if opts.MxBuildPath != "" { + return opts.MxBuildPath + } + if opts.Version != "" { + return CachedMxBuildPath(opts.Version) + } + return AnyCachedMxBuildPath() +} + +// noServeMxBuildError explains a failed resolution in the terms the user can act +// on. It names the version that was asked for and what the cache does hold, +// because the two being different is the whole defect — and it does not send a +// macOS user to `setup mxbuild`, which on a host with Studio Pro installed +// reports the path and caches nothing, leaving them exactly where they started +// (#1124). +func noServeMxBuildError(version string) error { + if version == "" { + return fmt.Errorf("mxbuild not found; run 'mxcli setup mxbuild -p ' or pass --mxbuild-path") + } + msg := fmt.Sprintf("no mxbuild for Mendix %s", version) + if cached := AnyCachedMxBuildPath(); cached != "" { + msg += fmt.Sprintf("\n The cache holds %s, which cannot build a %s project:\n %s", + versionFromPath(cached), version, cached) + } + return fmt.Errorf("%s\n"+ + " Install Mendix Studio Pro %[2]s (its bundled mxbuild is used automatically on macOS and Windows),\n"+ + " point mxcli at one with --mxbuild-path, or on Linux run 'mxcli setup mxbuild --version %[2]s'.", msg, version) +} + // StartServe launches `mxbuild --serve` and blocks until the build API responds. // Call Stop() to shut it down. The first Build() loads the model (cold, ~10-15s); // subsequent builds are incremental (~1s). func StartServe(opts ServeOptions) (*ServeServer, error) { - mxbuildPath := opts.MxBuildPath - if mxbuildPath == "" && opts.Version != "" { - mxbuildPath = CachedMxBuildPath(opts.Version) - } - if mxbuildPath == "" { - mxbuildPath = AnyCachedMxBuildPath() - } + mxbuildPath := resolveServeMxBuild(opts) if mxbuildPath == "" { - return nil, fmt.Errorf("mxbuild not found; run 'mxcli setup mxbuild -p ' or pass ServeOptions.MxBuildPath") + return nil, noServeMxBuildError(opts.Version) } if err := verifyMxBuildCache(mxbuildPath); err != nil { return nil, err diff --git a/cmd/mxcli/docker/mxserve_resolution_test.go b/cmd/mxcli/docker/mxserve_resolution_test.go new file mode 100644 index 0000000000..6bf8a39fc8 --- /dev/null +++ b/cmd/mxcli/docker/mxserve_resolution_test.go @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +// The local loop resolves mxbuild once (ResolveMxBuildForLocal) and then started +// mxbuild --serve from a SECOND resolver inside StartServe, which walked the +// cache on its own. The two answers differed, and the serve one won — so a Mac +// with Studio Pro installed built with whatever the cache happened to hold. +// These tests pin both halves of the fix. (issue #1122) + +// plantCacheEntry writes /.mxcli/mxbuild//modeler/mxbuild plus the +// runtime/ sibling verifyMxBuildCache requires, so resolution reaches its verdict +// rather than tripping the cache-completeness guard first. +func plantCacheEntry(t *testing.T, home, version string) string { + t.Helper() + base := filepath.Join(home, ".mxcli", "mxbuild", version) + if err := os.MkdirAll(filepath.Join(base, "runtime"), 0o755); err != nil { + t.Fatal(err) + } + return writeBinary(t, filepath.Join(base, "modeler"), "mxbuild", elfMagic) +} + +// TestStartServeRefusesVersionSubstitution is the reporter's shape 1. With no +// cache entry for the project's version, StartServe used to fall through to +// AnyCachedMxBuildPath — the NEWEST cached version, whatever it was — and hand it +// to mxbuild, which rejected it several minutes later with "Project version +// '11.12.2' does not exactly match MxBuild version '11.14.0'". +// +// A known version that is not cached must fail HERE, naming both versions. +func TestStartServeRefusesVersionSubstitution(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + plantCacheEntry(t, home, "11.14.0") + + _, err := StartServe(ServeOptions{Version: "11.12.2"}) + if err == nil { + t.Fatal("a cache holding only 11.14.0 must not satisfy a request for 11.12.2") + } + for _, want := range []string{"11.12.2", "11.14.0"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should name %q so the mismatch is visible; got:\n%v", want, err) + } + } +} + +// TestStartServeRefusesModelerlessCacheDir covers how the cache comes to be in +// that state, which is not an accident of the user's setup: step 2 of the same +// command creates // to hold the runtime symlink, and on macOS +// nothing ever puts a modeler/ beside it (mxbuild comes from Studio Pro). So the +// half-populated directory that defeats CachedMxBuildPath is mxcli's own work. +func TestStartServeRefusesModelerlessCacheDir(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + plantCacheEntry(t, home, "11.14.0") + + // What ensureMxBuildRuntimeSibling leaves behind for the project's version. + runtimeSrc := filepath.Join(home, ".mxcli", "runtime", "11.12.2", "runtime") + if err := os.MkdirAll(runtimeSrc, 0o755); err != nil { + t.Fatal(err) + } + if err := ensureMxBuildRuntimeSibling("11.12.2", io.Discard); err != nil { + t.Fatal(err) + } + if got := CachedMxBuildPath("11.12.2"); got != "" { + t.Fatalf("precondition: the cache dir must have no modeler entry, got %q", got) + } + + _, err := StartServe(ServeOptions{Version: "11.12.2"}) + if err == nil { + t.Fatal("a modeler-less cache dir must not silently resolve to another version") + } + // Asserting only that this fails proves nothing: the old code substituted + // 11.14.0 and then failed anyway, one exec later, with "exec format error". + // The verdict has to be reached HERE, naming the version that is missing. + if !strings.Contains(err.Error(), "no mxbuild for Mendix 11.12.2") { + t.Errorf("resolution must refuse up front, not fail later on a substituted binary; got:\n%v", err) + } +} + +// TestStartServeUsesAnyCachedOnlyWithoutAVersion keeps the fallback available +// where it is not a guess: a caller that names no version has nothing to be +// mismatched against. +func TestStartServeUsesAnyCachedOnlyWithoutAVersion(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + planted := plantCacheEntry(t, home, "11.14.0") + + if got := resolveServeMxBuild(ServeOptions{}); got != planted { + t.Errorf("with no version requested, resolution should fall back to the newest cached entry %q, got %q", planted, got) + } +} + +// TestServeOptionsForCarriesResolvedMxBuild is the call-site half. Both local +// entry points resolve mxbuild for this host and then build a ServeOptions; the +// bug was that the resolved path was dropped on the floor at that exact step +// (the web-client bundler received it, the build server did not). Constructing +// the options through one function is what stops the two from drifting again. +func TestServeOptionsForCarriesResolvedMxBuild(t *testing.T) { + opts := serveOptionsFor("/Applications/Mendix Studio Pro 11.12.2.app/Contents/modeler/mxbuild", "11.12.2", 21, 6543) + + if opts.MxBuildPath == "" { + t.Fatal("the resolved mxbuild must reach StartServe; an empty MxBuildPath sends it back to the cache") + } + if opts.Version != "11.12.2" || opts.JavaMajor != 21 || opts.Port != 6543 { + t.Errorf("unexpected options: %+v", opts) + } +} diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index ff86cea6d4..199fdbd8be 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -627,12 +627,7 @@ func RunLocal(opts LocalRunOptions) error { // 5. Start the warm build server. fmt.Fprintln(w, "Starting mxbuild --serve...") javaMajor, _ := ProjectJavaMajor(opts.ProjectPath) - serve, err := StartServe(ServeOptions{ - Version: version, - JavaMajor: javaMajor, - Host: "127.0.0.1", - Port: opts.ServePort, - }) + serve, err := StartServe(serveOptionsFor(mxbuildPath, version, javaMajor, opts.ServePort)) if err != nil { return fmt.Errorf("starting mxbuild serve: %w", err) } @@ -1130,6 +1125,13 @@ func clientBundleServedWithin(appURL string, window time.Duration) bool { // path the non-watch boot uses) and re-probe. A no-op when the bundle is already // served (a pure model reload never touches web/dist). func ensureClientServed(deployDir, appURL, mxbuildPath string, out io.Writer) error { + // Nothing below applies to the classic (Dojo) client: it has no bundle, no + // chunks, and never serves /dist/index.js. Fixing only the boot path would + // have moved this failure to every applied change under --watch rather than + // removing it (#1123). + if planWebClient(deployDir) == webClientClassic { + return nil + } // A dangling chunk is checked FIRST, because the index.js probe cannot see it: // the entry point is served with a 200 while a chunk it imports is missing, so // the apply is reported as successful and the page dies in the browser with diff --git a/cmd/mxcli/docker/testdata/webclient/.gitattributes b/cmd/mxcli/docker/testdata/webclient/.gitattributes new file mode 100644 index 0000000000..68f6c79ff1 --- /dev/null +++ b/cmd/mxcli/docker/testdata/webclient/.gitattributes @@ -0,0 +1,3 @@ +# Byte-for-byte as mxbuild writes them (CRLF included) — these are evidence, not +# source. Normalising them would quietly edit the fixture the tests are pinned to. +*.html -text diff --git a/cmd/mxcli/docker/testdata/webclient/README.md b/cmd/mxcli/docker/testdata/webclient/README.md new file mode 100644 index 0000000000..558442a92c --- /dev/null +++ b/cmd/mxcli/docker/testdata/webclient/README.md @@ -0,0 +1,19 @@ +# Web client entry points, as mxbuild writes them + +Both files are `deployment/web/index.html` from a real `mxbuild --target=deploy` +run of the same blank Mendix 11.12.2 app, differing only in +**Settings > Web UI > OptimizedClient**: + +| OptimizedClient | `web/` holds | `web/index.html` loads | sibling dir | +|---|---|---|---| +| `Yes` (default) | the React client | `dist/index.js` | `dojo-web/` | +| `No` | the classic Dojo client | `mxclientsystem/mxui/mxui.js` | `react-web/` | + +mxbuild swaps which client lands in `web/` and parks the other one beside it, so +the entry point names the client that is actually served. That is what +`planWebClient` reads, rather than the model's setting: a stale deployment and a +just-changed setting disagree, and it is the deployment that gets served. + +A classic deployment has **no** `web/rollup.config.mjs` and **no** `web/dist/` — +there is no bundling step for the Dojo client — which is why the gate that +assumed one of those two must exist rejected the app outright (issue #1123). diff --git a/cmd/mxcli/docker/testdata/webclient/classic-index.html b/cmd/mxcli/docker/testdata/webclient/classic-index.html new file mode 100644 index 0000000000..39199f3d84 --- /dev/null +++ b/cmd/mxcli/docker/testdata/webclient/classic-index.html @@ -0,0 +1,46 @@ + + + + + + Mendix + + + + + + + + + + +
+ + + + + diff --git a/cmd/mxcli/docker/testdata/webclient/optimized-index.html b/cmd/mxcli/docker/testdata/webclient/optimized-index.html new file mode 100644 index 0000000000..5a11735e31 --- /dev/null +++ b/cmd/mxcli/docker/testdata/webclient/optimized-index.html @@ -0,0 +1,40 @@ + + + + + + + Mendix + + + + + + + + + + +
+ + + + \ No newline at end of file diff --git a/cmd/mxcli/docker/version_resolve.go b/cmd/mxcli/docker/version_resolve.go new file mode 100644 index 0000000000..251cd8f9d5 --- /dev/null +++ b/cmd/mxcli/docker/version_resolve.go @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "encoding/xml" + "fmt" + "io" + "net/http" + "sort" + "strconv" + "strings" +) + +// The Mendix CDN is an S3 bucket that answers the ListObjectsV2 API, which is +// how a partial version is turned into the artifact that actually exists. +const ( + cdnBucketURL = "https://cdn.mendix.com/" + cdnKeyPrefix = "runtime/" + cdnListMaxKey = 1000 +) + +// ResolveCDNVersion maps a user-supplied Mendix version onto the version string +// the CDN publishes an artifact for. +// +// Mendix 11 publishes three-part names (mxbuild-11.13.0.tar.gz). Mendix 9 and 10 +// publish FOUR parts, with a build number: the release called "10.24.25" is +// mxbuild-10.24.25.122571.tar.gz. A three-part 10.x version therefore names no +// artifact at all, and every probe of one 404s — which reads as "Mendix 10 is not +// on the CDN" rather than "that is not its name". Both the mxbuild and the +// runtime archive follow this, so one resolved string serves both. +// +// A version that already carries a build number is returned untouched, and so is +// one the CDN serves as given — the listing call only happens when a download +// would otherwise fail. A project-derived version needs none of this: the MPR's +// _ProductVersion already carries all four parts. +// +// Where several builds exist for one release (10.24.24 has three), the highest +// build number wins: they are respins of the same release and the last one is +// what Mendix ships. +func ResolveCDNVersion(version, goarch string) (string, error) { + version = strings.TrimSpace(version) + if version == "" { + return "", fmt.Errorf("empty Mendix version") + } + // Already exact — a build number is present. + if len(strings.Split(version, ".")) >= 4 { + return version, nil + } + // Published under the name as given (every supported Mendix 11). + if cdnHasArtifact(MxBuildCDNURL(version, goarch)) { + return version, nil + } + + stem := mxbuildKeyStem(goarch) + keys, err := listCDNKeys(cdnKeyPrefix + stem + version + ".") + if err != nil { + return "", fmt.Errorf("resolving Mendix %s against the CDN: %w", version, err) + } + resolved := highestBuild(keys, stem, version) + if resolved == "" { + return "", fmt.Errorf("no MxBuild archive published for Mendix %s "+ + "(tried %s and a CDN listing of %s*)", + version, MxBuildCDNURL(version, goarch), stem+version+".") + } + return resolved, nil +} + +// mxbuildKeyStem is the filename stem for an architecture, matching MxBuildCDNURL. +func mxbuildKeyStem(goarch string) string { + if goarch == "arm64" { + return "arm64-mxbuild-" + } + return "mxbuild-" +} + +// cdnHasArtifact reports whether the CDN serves the given URL. A transport error +// or any non-200 counts as absent: the caller falls back to a listing, and a real +// outage surfaces there with a better message than a HEAD failure would give. +func cdnHasArtifact(url string) bool { + resp, err := http.Head(url) + if err != nil { + return false + } + defer resp.Body.Close() + return resp.StatusCode == http.StatusOK +} + +// s3ListResult is the subset of ListObjectsV2 output this needs. +type s3ListResult struct { + IsTruncated bool `xml:"IsTruncated"` + NextContinuationToken string `xml:"NextContinuationToken"` + Contents []struct { + Key string `xml:"Key"` + } `xml:"Contents"` +} + +// listCDNKeys returns every object key under a prefix, following continuation +// tokens. The prefixes used here match a handful of keys, but a truncated +// response that was treated as complete would silently resolve to the wrong +// build, so the loop is not optional. +func listCDNKeys(prefix string) ([]string, error) { + var keys []string + token := "" + for { + url := fmt.Sprintf("%s?list-type=2&prefix=%s&max-keys=%d", + cdnBucketURL, prefix, cdnListMaxKey) + if token != "" { + url += "&continuation-token=" + token + } + resp, err := http.Get(url) + if err != nil { + return nil, err + } + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("HTTP %d listing %s", resp.StatusCode, prefix) + } + parsed, err := parseCDNListing(body) + if err != nil { + return nil, err + } + keys = append(keys, parsed.keys...) + if !parsed.truncated || parsed.next == "" { + return keys, nil + } + token = parsed.next + } +} + +type cdnListing struct { + keys []string + truncated bool + next string +} + +// parseCDNListing extracts the object keys from one ListObjectsV2 response. +func parseCDNListing(body []byte) (cdnListing, error) { + var res s3ListResult + if err := xml.Unmarshal(body, &res); err != nil { + return cdnListing{}, fmt.Errorf("parsing CDN listing: %w", err) + } + out := cdnListing{truncated: res.IsTruncated, next: res.NextContinuationToken} + for _, c := range res.Contents { + out.keys = append(out.keys, c.Key) + } + return out, nil +} + +// highestBuild picks the highest-numbered build of one release from a set of +// object keys, and returns the full four-part version. +// +// It matches `..tar.gz` exactly: the `.sha256` sidecar +// next to every archive must not be mistaken for an artifact, and a prefix +// without the trailing dot would let 10.24.2 match 10.24.20 through 10.24.26. +// Comparison is numeric, because build numbers are not zero-padded and sorting +// them as text puts 99999 above 122571. +func highestBuild(keys []string, stem, version string) string { + want := stem + version + "." + best := -1 + for _, key := range keys { + name := key + if i := strings.LastIndex(name, "/"); i >= 0 { + name = name[i+1:] + } + if !strings.HasPrefix(name, want) || !strings.HasSuffix(name, ".tar.gz") { + continue + } + build := strings.TrimSuffix(strings.TrimPrefix(name, want), ".tar.gz") + n, err := strconv.Atoi(build) + if err != nil { + continue // not a build number (e.g. a suffixed variant) + } + if n > best { + best = n + } + } + if best < 0 { + return "" + } + return version + "." + strconv.Itoa(best) +} + +// CDNReleasesFor lists the full versions published for a partial one, newest +// first. Used to show what was available when a version cannot be resolved. +func CDNReleasesFor(partial, goarch string) []string { + stem := mxbuildKeyStem(goarch) + keys, err := listCDNKeys(cdnKeyPrefix + stem + partial) + if err != nil { + return nil + } + seen := map[string]bool{} + var out []string + for _, key := range keys { + name := key + if i := strings.LastIndex(name, "/"); i >= 0 { + name = name[i+1:] + } + if !strings.HasPrefix(name, stem) || !strings.HasSuffix(name, ".tar.gz") { + continue + } + v := strings.TrimSuffix(strings.TrimPrefix(name, stem), ".tar.gz") + if !seen[v] { + seen[v] = true + out = append(out, v) + } + } + sortCDNReleases(out) + return out +} + +// sortCDNReleases orders versions newest first, numerically. Sorting them as +// text puts 10.24.9 above 10.24.26, so the hint would name a release two dozen +// patches old as the latest. +func sortCDNReleases(versions []string) { + sort.Slice(versions, func(i, j int) bool { + li, oki := parseVersionParts(versions[i]) + lj, okj := parseVersionParts(versions[j]) + if !oki || !okj { + return versions[i] > versions[j] + } + return compareVersionParts(li, lj) > 0 + }) +} diff --git a/cmd/mxcli/docker/version_resolve_test.go b/cmd/mxcli/docker/version_resolve_test.go new file mode 100644 index 0000000000..e502265fa9 --- /dev/null +++ b/cmd/mxcli/docker/version_resolve_test.go @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import "testing" + +// A real ListObjectsV2 response for prefix "runtime/mxbuild-10.24.24." — the +// release Mendix respun three times, which is what makes "pick a build" a +// decision rather than a lookup. Every archive is shadowed by a .sha256 sidecar. +const listing10_24_24 = ` + +mx-cdnruntime/mxbuild-10.24.24. +620false +runtime/mxbuild-10.24.24.119349.tar.gz +runtime/mxbuild-10.24.24.119349.tar.gz.sha256 +runtime/mxbuild-10.24.24.119564.tar.gz +runtime/mxbuild-10.24.24.119564.tar.gz.sha256 +runtime/mxbuild-10.24.24.119653.tar.gz +runtime/mxbuild-10.24.24.119653.tar.gz.sha256 +` + +// A truncated page, to prove the continuation token is read rather than ignored. +const listingTruncated = ` + +mx-cdnruntime/mxbuild-10.24 +14OiJwODsCgR7yYEMps95YFPhA5XOi74 +11true +runtime/mxbuild-10.24.0.72725.tar.gz +` + +func TestParseCDNListing(t *testing.T) { + got, err := parseCDNListing([]byte(listing10_24_24)) + if err != nil { + t.Fatalf("parseCDNListing: %v", err) + } + if len(got.keys) != 6 { + t.Errorf("keys = %d, want 6: %v", len(got.keys), got.keys) + } + if got.truncated { + t.Error("truncated = true, want false") + } + + tr, err := parseCDNListing([]byte(listingTruncated)) + if err != nil { + t.Fatalf("parseCDNListing(truncated): %v", err) + } + if !tr.truncated { + t.Error("truncated = false, want true — a page treated as complete resolves to the wrong build") + } + if tr.next != "14OiJwODsCgR7yYEMps95YFPhA5XOi74" { + t.Errorf("next = %q, want the continuation token", tr.next) + } +} + +func TestHighestBuild(t *testing.T) { + listing, err := parseCDNListing([]byte(listing10_24_24)) + if err != nil { + t.Fatalf("parseCDNListing: %v", err) + } + + if got := highestBuild(listing.keys, "mxbuild-", "10.24.24"); got != "10.24.24.119653" { + t.Errorf("highestBuild = %q, want 10.24.24.119653 (the last respin)", got) + } + + // The .sha256 sidecar sits next to every archive and must never be chosen. + only := []string{"runtime/mxbuild-10.24.25.122571.tar.gz.sha256"} + if got := highestBuild(only, "mxbuild-", "10.24.25"); got != "" { + t.Errorf("highestBuild on a sidecar alone = %q, want empty", got) + } + + // Build numbers are not zero-padded, so text sorting would put 99999 first. + unpadded := []string{ + "runtime/mxbuild-10.24.25.99999.tar.gz", + "runtime/mxbuild-10.24.25.122571.tar.gz", + } + if got := highestBuild(unpadded, "mxbuild-", "10.24.25"); got != "10.24.25.122571" { + t.Errorf("highestBuild = %q, want 10.24.25.122571 — compare build numbers numerically, not as text", got) + } + + // Without the trailing dot, 10.24.2 would swallow 10.24.20..10.24.26. The + // prefix highestBuild builds must keep releases apart. + neighbours := []string{ + "runtime/mxbuild-10.24.2.75382.tar.gz", + "runtime/mxbuild-10.24.20.105674.tar.gz", + "runtime/mxbuild-10.24.25.122571.tar.gz", + } + if got := highestBuild(neighbours, "mxbuild-", "10.24.2"); got != "10.24.2.75382" { + t.Errorf("highestBuild = %q, want 10.24.2.75382 — 10.24.20 and 10.24.25 are different releases", got) + } + + // arm64 archives carry their own stem; the amd64 stem must not match them. + arm := []string{"runtime/arm64-mxbuild-10.24.25.122571.tar.gz"} + if got := highestBuild(arm, "mxbuild-", "10.24.25"); got != "" { + t.Errorf("amd64 stem matched an arm64 key: %q", got) + } + if got := highestBuild(arm, "arm64-mxbuild-", "10.24.25"); got != "10.24.25.122571" { + t.Errorf("arm64 stem = %q, want 10.24.25.122571", got) + } +} + +func TestMxbuildKeyStemMatchesURL(t *testing.T) { + // The listing prefix and the download URL must name the same artifact, or a + // resolved version points at a file the downloader will not ask for. + for _, arch := range []string{"amd64", "arm64"} { + url := MxBuildCDNURL("10.24.25.122571", arch) + want := "https://cdn.mendix.com/runtime/" + mxbuildKeyStem(arch) + "10.24.25.122571.tar.gz" + if url != want { + t.Errorf("arch %s: URL %q, stem builds %q", arch, url, want) + } + } +} + +// A version that already carries a build number must be returned untouched and +// must not reach the network — this is the common path for a project-derived +// version, where _ProductVersion is already four parts. +func TestResolveCDNVersionPassesThroughFourPart(t *testing.T) { + got, err := ResolveCDNVersion("10.24.25.122571", "amd64") + if err != nil { + t.Fatalf("ResolveCDNVersion: %v", err) + } + if got != "10.24.25.122571" { + t.Errorf("= %q, want it returned unchanged", got) + } +} + +func TestResolveCDNVersionRejectsEmpty(t *testing.T) { + if _, err := ResolveCDNVersion(" ", "amd64"); err == nil { + t.Error("empty version accepted; want an error") + } +} + +// The "newest first" hint shown when a version cannot be resolved must order +// releases numerically. Sorted as text, 10.24.9 outranks 10.24.26 and the hint +// names a release two dozen patches old as the latest. +func TestCDNReleasesSortIsNumeric(t *testing.T) { + in := []string{"10.24.9.81004", "10.24.26.123458", "10.24.2.75382", "10.24.20.105674"} + got := append([]string(nil), in...) + sortCDNReleases(got) + want := []string{"10.24.26.123458", "10.24.20.105674", "10.24.9.81004", "10.24.2.75382"} + for i := range want { + if got[i] != want[i] { + t.Fatalf("sorted = %v, want %v", got, want) + } + } +} diff --git a/cmd/mxcli/docker/webclient.go b/cmd/mxcli/docker/webclient.go index bbe82fe0fb..4cf7f078f7 100644 --- a/cmd/mxcli/docker/webclient.go +++ b/cmd/mxcli/docker/webclient.go @@ -101,32 +101,23 @@ func BuildWebClient(opts WebClientOptions) error { w = io.Discard } webDir := filepath.Join(opts.DeployDir, "web") - if fi, err := os.Stat(filepath.Join(webDir, "rollup.config.mjs")); err != nil || fi.IsDir() { + switch plan := planWebClient(opts.DeployDir); plan { + case webClientPrebuilt: // Mendix 11.14 closed this gap upstream: its build writes web/dist/ // itself and no longer emits a rollup config, because there is nothing - // left to configure. Measured on a blank app, same build target: - // - // 11.13.0 rollup.config.mjs PRESENT dist/index.js ABSENT - // 11.14.0 rollup.config.mjs ABSENT dist/index.js PRESENT - // - // The old gate tested for the config, so on 11.14 it failed on the - // absence of a file whose purpose had been served — fatally, at both - // call sites, for EVERY 11.14 app (ako/mxcli-ledger #146). - // - // Gate on the gap instead of on the shape one version happened to leave - // behind: if the bundle is already there, this step has nothing to do. - // The config is still required when it is NOT there, because then the - // rollup run is the only thing that can produce it. - if WebClientBundled(opts.DeployDir) { - fmt.Fprintln(w, " Web client already bundled by mxbuild; skipping rollup step") - return nil - } - return fmt.Errorf("no rollup.config.mjs and no bundle at %s\n"+ - " Mendix 11.13 and earlier emit a rollup config for mxcli to run; 11.14+ writes\n"+ - " the bundle itself. Neither is present, so the build did not produce a client:\n"+ - " run a serve Deploy build first (or delete deployment/ if it was built by an\n"+ - " older Mendix version).", - webClientBundlePath(opts.DeployDir)) + // left to configure (ako/mxcli-ledger #146). + fmt.Fprintln(w, " Web client already bundled by mxbuild; skipping rollup step") + return nil + case webClientClassic: + // Settings > Web UI > OptimizedClient = No. The deployment's client is + // the Dojo one under web/, loaded from mxclientsystem and served as-is; + // there is no bundle to build and never was. Refusing here failed the + // whole run, claiming the build had produced no client while its client + // sat in the same directory (#1123). + fmt.Fprintln(w, " Classic (Dojo) web client — no bundling step; skipping rollup") + return nil + case webClientMissing: + return noWebClientError(opts.DeployDir) } nodeBin, runner, err := resolveNodeTooling(opts.MxBuildPath) if err != nil { @@ -232,6 +223,12 @@ func ensureWebClientBundle(deployDir string, w io.Writer, bundle func() error) ( if w == nil { w = io.Discard } + // A classic-client deployment has no bundle and never will, so "the bundle is + // missing" is its steady state rather than damage from the boot. Without this + // the guard would re-run the bundler after every boot of such an app. + if planWebClient(deployDir) == webClientClassic { + return false, nil + } if WebClientBundled(deployDir) { return false, nil } diff --git a/cmd/mxcli/docker/webclient_classic_test.go b/cmd/mxcli/docker/webclient_classic_test.go new file mode 100644 index 0000000000..18c1e94eec --- /dev/null +++ b/cmd/mxcli/docker/webclient_classic_test.go @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "bytes" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +// A project with Settings > Web UI > OptimizedClient = No deploys Mendix's +// classic (Dojo) client, which has no bundling step at all: no +// web/rollup.config.mjs and no web/dist. The gate assumed one of those two must +// exist and failed the run outright, claiming "the build did not produce a +// client" about a deployment that had one. (issue #1123) + +// classicDeployment builds a deployment directory shaped like a real +// OptimizedClient=No build: web/index.html from mxbuild, no rollup config, no +// dist, and the React client parked in react-web/. +func classicDeployment(t *testing.T) string { + t.Helper() + dir := t.TempDir() + webDir := filepath.Join(dir, "web") + if err := os.MkdirAll(filepath.Join(dir, "react-web"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(webDir, 0o755); err != nil { + t.Fatal(err) + } + src, err := os.ReadFile(filepath.Join("testdata", "webclient", "classic-index.html")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(webDir, "index.html"), src, 0o644); err != nil { + t.Fatal(err) + } + // The React client's rollup config lives in the PARKED directory, not in + // web/. Planting it proves the plan reads web/ and is not fooled by it. + if err := os.WriteFile(filepath.Join(dir, "react-web", "rollup.config.mjs"), []byte("export default {}"), 0o644); err != nil { + t.Fatal(err) + } + return dir +} + +// optimizedDeployment is the control: the same app with OptimizedClient = Yes, +// where web/ holds the React client and its rollup config. +func optimizedDeployment(t *testing.T) string { + t.Helper() + dir := t.TempDir() + webDir := filepath.Join(dir, "web") + if err := os.MkdirAll(webDir, 0o755); err != nil { + t.Fatal(err) + } + src, err := os.ReadFile(filepath.Join("testdata", "webclient", "optimized-index.html")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(webDir, "index.html"), src, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(webDir, "rollup.config.mjs"), []byte("export default {}"), 0o644); err != nil { + t.Fatal(err) + } + return dir +} + +// TestPlanWebClientReadsRealEntryPoints pins the decision against the actual +// index.html files mxbuild writes for both settings of the same 11.12.2 app — +// the classic one is the deployment the reporter had. +func TestPlanWebClientReadsRealEntryPoints(t *testing.T) { + if got := planWebClient(classicDeployment(t)); got != webClientClassic { + t.Errorf("a deployment whose web/index.html loads mxclientsystem is the classic client, got %v", got) + } + if got := planWebClient(optimizedDeployment(t)); got != webClientRollup { + t.Errorf("a React deployment with a rollup config needs the rollup step, got %v", got) + } +} + +// TestPlanWebClientPrebuiltAndMissing keeps the two pre-existing outcomes intact: +// Mendix 11.14+ writes web/dist itself, and a deployment with none of the three +// shapes is still the genuine "no client" failure this gate exists for. +func TestPlanWebClientPrebuiltAndMissing(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "web", "dist"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "web", "dist", "index.js"), []byte("console.log(1)"), 0o644); err != nil { + t.Fatal(err) + } + if got := planWebClient(dir); got != webClientPrebuilt { + t.Errorf("11.14+ writes the bundle itself, got %v", got) + } + + if got := planWebClient(t.TempDir()); got != webClientMissing { + t.Errorf("an empty deployment really has no client, got %v", got) + } +} + +// TestBuildWebClientSkipsClassic is the reporter's failure. With no rollup config +// and no dist, BuildWebClient errored — so `mxcli run --local` exited 1 after a +// ~154s cold build, saying the build produced no client about a deployment whose +// client was sitting in the same directory. +func TestBuildWebClientSkipsClassic(t *testing.T) { + var out bytes.Buffer + // No MxBuildPath: reaching the node tooling would be a failure in itself, + // since there is nothing for rollup to do. + if err := BuildWebClient(WebClientOptions{DeployDir: classicDeployment(t), Stdout: &out}); err != nil { + t.Fatalf("a classic-client deployment must not be treated as a failed build: %v", err) + } + if !strings.Contains(strings.ToLower(out.String()), "classic") { + t.Errorf("the skip should say why it did nothing, got: %q", out.String()) + } +} + +// TestBuildWebClientStillRejectsAClientlessDeployment is the control for the one +// above: the gate must keep failing where it was right. Without it, "skip when +// there is nothing to bundle" degenerates into "never report a broken build". +func TestBuildWebClientStillRejectsAClientlessDeployment(t *testing.T) { + err := BuildWebClient(WebClientOptions{DeployDir: t.TempDir(), Stdout: io.Discard}) + if err == nil { + t.Fatal("a deployment with no client at all must still be reported") + } + if !strings.Contains(err.Error(), "did not produce a client") { + t.Errorf("unexpected error: %v", err) + } +} + +// TestStartWebClientWatchSkipsClassic — under --watch the second copy of the gate +// applied, so `run --local --watch` failed on a classic app even when the boot +// path was fixed. A nil watcher is the existing "no bundler to keep hot" signal +// and every method on it is nil-safe. +func TestStartWebClientWatchSkipsClassic(t *testing.T) { + var out bytes.Buffer + w, err := StartWebClientWatch(WebClientOptions{DeployDir: classicDeployment(t), Stdout: &out}) + if err != nil { + t.Fatalf("classic deployment must not fail the watcher: %v", err) + } + if w != nil { + t.Fatal("there is no incremental bundler for the classic client") + } + w.Stop() // nil-safe, as the loop relies on +} + +// TestEnsureWebClientBundleSkipsClassic — the post-boot guard re-bundles when +// web/dist is missing, which for a classic app is always, so it would have +// re-run the bundler on every boot. +func TestEnsureWebClientBundleSkipsClassic(t *testing.T) { + called := false + rebuilt, err := ensureWebClientBundle(classicDeployment(t), io.Discard, func() error { + called = true + return nil + }) + if err != nil { + t.Fatalf("classic deployment: %v", err) + } + if called || rebuilt { + t.Error("nothing to re-bundle: the classic client has no bundle") + } +} + +// TestEnsureClientServedSkipsClassic — the per-apply guard asserts /dist/index.js +// is being SERVED, which a classic app never serves. Under --watch this fails on +// every applied change, so fixing only the boot path would have moved the failure +// rather than removed it. appURL is deliberately unreachable: reaching the probe +// at all is the defect. +func TestEnsureClientServedSkipsClassic(t *testing.T) { + if err := ensureClientServed(classicDeployment(t), "http://127.0.0.1:1", "", io.Discard); err != nil { + t.Fatalf("classic deployment must not be probed for a bundle it has no concept of: %v", err) + } +} diff --git a/cmd/mxcli/docker/webclient_plan.go b/cmd/mxcli/docker/webclient_plan.go new file mode 100644 index 0000000000..6c3401bd89 --- /dev/null +++ b/cmd/mxcli/docker/webclient_plan.go @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" +) + +// webClientPlan says what, if anything, mxcli must do to give a deployment a +// browser client. +// +// It exists because the question was answered in two places with the same +// hand-written pair of file checks — BuildWebClient and StartWebClientWatch — +// and they drifted: the Mendix 11.14 fix (ako/mxcli-ledger #146) landed on the +// first, so `run --local` started working on 11.14 while `run --local --watch` +// kept failing. Adding the classic-client case to both would have made three +// copies of a gate that has already drifted once, so the decision is made here +// and the call sites switch on it. +type webClientPlan int + +// Measured on blank apps at the same build target: +// +// 11.13.0, OptimizedClient=Yes web/rollup.config.mjs PRESENT web/dist/index.js ABSENT +// 11.14.0, OptimizedClient=Yes web/rollup.config.mjs ABSENT web/dist/index.js PRESENT +// 11.12.2, OptimizedClient=No web/rollup.config.mjs ABSENT web/dist/index.js ABSENT +// web/index.html loads mxclientsystem/mxui/mxui.js +// +// The third row is why absence cannot be the test for either of the other two: +// it looks identical to a build that produced nothing. +const ( + // webClientRollup: mxbuild wrote the client source and a rollup config but + // not the bundle. Mendix 11.13 and earlier. mxcli runs the rollup step. + webClientRollup webClientPlan = iota + // webClientPrebuilt: mxbuild wrote web/dist itself. Mendix 11.14+, which + // emits no rollup config because there is nothing left to configure. + webClientPrebuilt + // webClientClassic: web/ holds the classic (Dojo) client, which has no + // bundling step in any Mendix version. Settings > Web UI > OptimizedClient + // = No. + webClientClassic + // webClientMissing: none of the above — the build genuinely produced no + // client, which is the case this gate was written for. + webClientMissing +) + +func (p webClientPlan) String() string { + switch p { + case webClientRollup: + return "rollup" + case webClientPrebuilt: + return "prebuilt" + case webClientClassic: + return "classic" + default: + return "missing" + } +} + +// classicClientEntryMarker is what the classic client's entry point loads. The +// React client's loads dist/index.js instead, so the file names which client the +// deployment will actually serve. +const classicClientEntryMarker = "mxclientsystem/mxui/mxui.js" + +// maxEntryPointRead bounds the read of web/index.html. Real ones are ~2KB; the +// cap is only so a wrong path cannot pull a large file into memory. +const maxEntryPointRead = 1 << 20 + +// planWebClient decides from the DEPLOYMENT, not from the model's +// OptimizedClient setting. +// +// The deployment is what gets served, and the two disagree exactly when it +// matters: right after the setting is changed, the old client is still on disk. +// Reading it here also needs no plumbing through the five call sites that ask +// this question, and it covers MigrationMode without having to predict which +// client that mode leaves in web/ — whichever one is there is the one to serve. +// +// mxbuild swaps the two clients in and out of web/ and parks the other beside it +// (dojo-web/ or react-web/), so only web/ is consulted; the parked directory has +// its own rollup config and is not what the app loads. +func planWebClient(deployDir string) webClientPlan { + webDir := filepath.Join(deployDir, "web") + + // The classic client is checked FIRST and on positive evidence. Inferring it + // from the absence of the React shapes would make every genuinely broken + // deployment look classic, and silently skipping the bundle for those is the + // black screen this whole file exists to prevent. + if isClassicWebClient(webDir) { + return webClientClassic + } + if fi, err := os.Stat(filepath.Join(webDir, "rollup.config.mjs")); err == nil && !fi.IsDir() { + return webClientRollup + } + if WebClientBundled(deployDir) { + return webClientPrebuilt + } + return webClientMissing +} + +// isClassicWebClient reports whether web/index.html loads the Dojo client. +func isClassicWebClient(webDir string) bool { + f, err := os.Open(filepath.Join(webDir, "index.html")) + if err != nil { + return false + } + defer f.Close() + b, err := io.ReadAll(io.LimitReader(f, maxEntryPointRead)) + if err != nil { + return false + } + return bytes.Contains(b, []byte(classicClientEntryMarker)) +} + +// noWebClientError is the verdict for a deployment that really has no client. +func noWebClientError(deployDir string) error { + return fmt.Errorf("no rollup.config.mjs and no bundle at %s\n"+ + " Mendix 11.13 and earlier emit a rollup config for mxcli to run; 11.14+ writes\n"+ + " the bundle itself; a classic-client app (Web UI Settings > OptimizedClient = No)\n"+ + " needs neither. None of the three is present, so the build did not produce a client:\n"+ + " run a serve Deploy build first (or delete deployment/ if it was built by an\n"+ + " older Mendix version).", + webClientBundlePath(deployDir)) +} diff --git a/cmd/mxcli/docker/webclient_watch.go b/cmd/mxcli/docker/webclient_watch.go index daa7fdc9a1..163bd6450a 100644 --- a/cmd/mxcli/docker/webclient_watch.go +++ b/cmd/mxcli/docker/webclient_watch.go @@ -138,30 +138,23 @@ func StartWebClientWatch(opts WebClientOptions) (*WebClientWatcher, error) { w = io.Discard } webDir := filepath.Join(opts.DeployDir, "web") - if fi, err := os.Stat(filepath.Join(webDir, "rollup.config.mjs")); err != nil || fi.IsDir() { - // The Mendix 11.14 shape, and the second copy of the gate that made - // mxcli unable to start any 11.14 app (ako/mxcli-ledger #146). That fix - // landed on BuildWebClient only, so `run --local` started working and - // `run --local --watch` kept failing on the absence of a file whose - // purpose 11.14 had served: - // - // 11.13.0 rollup.config.mjs PRESENT dist/index.js ABSENT - // 11.14.0 rollup.config.mjs ABSENT dist/index.js PRESENT - // - // When mxbuild bundles the client itself there is no incremental - // bundler to keep hot and nothing for it to do. A nil watcher says - // exactly that, and the loop treats it as "the serve build produces - // web/dist" — with ensureClientServed still guarding the result. - if WebClientBundled(opts.DeployDir) { - fmt.Fprintln(w, " Web client bundled by mxbuild; no incremental bundler needed") - return nil, nil - } - return nil, fmt.Errorf("no rollup.config.mjs and no bundle at %s\n"+ - " Mendix 11.13 and earlier emit a rollup config for mxcli to run; 11.14+ writes\n"+ - " the bundle itself. Neither is present, so the build did not produce a client:\n"+ - " run a serve Deploy build first (or delete deployment/ if it was built by an\n"+ - " older Mendix version).", - webClientBundlePath(opts.DeployDir)) + // This used to carry its own copy of BuildWebClient's gate, which is how the + // 11.14 fix reached one and not the other — `run --local` worked and `run + // --local --watch` did not. Both now switch on the same planWebClient. + // + // A nil watcher means "there is no incremental bundler to keep hot", which is + // true both when mxbuild writes web/dist itself and when the deployment is the + // classic client. Every watcher method is nil-safe, so the loop needs no + // branch, and ensureClientServed still guards the result. + switch plan := planWebClient(opts.DeployDir); plan { + case webClientPrebuilt: + fmt.Fprintln(w, " Web client bundled by mxbuild; no incremental bundler needed") + return nil, nil + case webClientClassic: + fmt.Fprintln(w, " Classic (Dojo) web client — no incremental bundler needed") + return nil, nil + case webClientMissing: + return nil, noWebClientError(opts.DeployDir) } nodeBin, runner, err := resolveNodeTooling(opts.MxBuildPath) if err != nil { diff --git a/cmd/mxcli/init_claudemd.go b/cmd/mxcli/init_claudemd.go index 5ab57e91d1..7d53060b9b 100644 --- a/cmd/mxcli/init_claudemd.go +++ b/cmd/mxcli/init_claudemd.go @@ -43,6 +43,66 @@ func extractSkillDescription(content []byte) string { return "MDL skill" } +// projectGate is one command in the ordered gate list the generated CLAUDE.md +// publishes as a project's definition of done. +// +// It exists as data rather than as literal markdown lines because the same +// sequence is stated in three places that must agree: this file (re-read into +// every session, so it IS the default behaviour), the `bootstrap-app` skill +// (which sets the project up and hands over), and the bootstrap-prompt docs +// page that describes the skill. When they were three hand-kept copies, +// `mxcli test` was named in the skill's ports note and on no gate list at all, +// so testing was reachable only by a user asking for it by name. +// TestBootstrapProcedureNamesEveryGate holds the three together. +type projectGate struct { + // Cmd is the command as written, without the leading "./mxcli ". + // A %s is substituted with the project's .mpr path. + Cmd string + // Note is the trailing comment explaining what the gate buys and roughly + // what it costs. + Note string + // Ref is the substring that must also appear in the bootstrap skill and on + // the docs page for this gate to count as stated there. + Ref string + // Command is the cobra command path ("docker check" for a subcommand), + // checked to exist so a rename breaks a test rather than every project's + // onboarding. + Command string +} + +// projectGates is the ordered, cheapest-first gate list. Adding one here adds +// it to every generated CLAUDE.md, and requires naming it in the bootstrap +// skill and docs page too. +var projectGates = []projectGate{ + {"check script.mdl -p %s --references", "syntax + references (~2s)", "mxcli check", "check"}, + {"exec script.mdl -p %s", "apply", "mxcli exec", "exec"}, + {"lint -p %s", "rules (~3s)", "mxcli lint", "lint"}, + {"report -p %s", "scored quality report", "mxcli report", "report"}, + {"docker check -p %s", "mxbuild, the slow one (~25s)", "mxcli docker check", "docker check"}, + {"test tests/ -p %s --local", "microflow tests (~30s cold, ~2s warm)", "mxcli test", "test"}, + {"run --local --watch -p %s", "the app, hot-reloading", "mxcli run --local", "run"}, +} + +// renderProjectGates writes the gate list as aligned shell lines, comments in +// one column. +func renderProjectGates(mprPath string) string { + lines := make([]string, len(projectGates)) + width := 0 + for i, g := range projectGates { + lines[i] = "./mxcli " + fmt.Sprintf(g.Cmd, mprPath) + if len(lines[i]) > width { + width = len(lines[i]) + } + } + var sb strings.Builder + for i, g := range projectGates { + sb.WriteString(lines[i]) + sb.WriteString(strings.Repeat(" ", width-len(lines[i]))) + sb.WriteString(" # " + g.Note + "\n") + } + return sb.String() +} + func generateClaudeMD(projectName, mprFile string) string { mprPath := mprFile if mprPath == "" { @@ -110,19 +170,21 @@ func generateClaudeMD(projectName, mprFile string) string { // ── The gates ─────────────────────────────────────────────────── // Ordered cheapest-first on purpose: each one is only worth paying for // once the one above it is clean. + // + // The list is projectGates rather than literal lines because the same set + // has to appear in the bootstrap procedure (the skill) and on the docs + // page that describes it. Three hand-kept copies is how `test` came to be + // in two of them and absent from the one that is re-read every session. w("## The gates, in order\n\n") - w("Run them cheapest-first; each is only worth paying for once the one above is clean.\n\n") + w("Run them cheapest-first; each is only worth paying for once the one above is clean.\n") + w("**They are the definition of done, not a menu** — a change is finished when they have\n") + w("all been run and you have said what each one reported.\n\n") w(bt3 + "bash\n") - w("./mxcli check script.mdl -p " + mprPath + " --references # syntax + references (~2s)\n") - w("./mxcli exec script.mdl -p " + mprPath + " # apply\n") - w("./mxcli lint -p " + mprPath + " # rules (~3s)\n") - w("./mxcli report -p " + mprPath + " # scored best practices\n") - w("./mxcli docker check -p " + mprPath + " # mxbuild, the slow one (~25s)\n") - w("./mxcli run --local --watch -p " + mprPath + " # the app, hot-reloading\n") + w(renderProjectGates(mprPath)) w(bt3 + "\n\n") w("**" + bt + "lint" + bt + " printing no errors is not a pass** — read the warning count, and read\n") - w(bt + "report" + bt + "'s score. A green " + bt + "check" + bt + " proves nothing about how a page renders:\n") - w("anything visual or stateful needs the app actually running.\n\n") + w(bt + "report" + bt + "'s score. A green " + bt + "check" + bt + " proves nothing about behaviour or about how\n") + w("a page renders: logic needs " + bt + "test" + bt + ", and anything visual needs the app actually running.\n\n") w("Set " + bt + "mx" + bt + " up once with " + bt + "./mxcli setup mxbuild -p " + mprPath + bt + ". To call it directly,\n") w("name the version — " + bt + "~/.mxcli/mxbuild//modeler/mx" + bt + " — because a " + bt + "*" + bt + " glob\n") w("breaks the moment two are cached.\n\n") @@ -155,5 +217,20 @@ func generateClaudeMD(projectName, mprFile string) string { w("- **" + bt + "@Position(x, y)" + bt + " is optional** — mxcli places microflow activities, and\n") w(" " + bt + "./mxcli layout" + bt + " arranges the domain model.\n\n") + // ── Finishing a change ────────────────────────────────────────── + // The bootstrap created README.md, FINDINGS.md and the plan; without this + // they are written once and then quietly stop being true, and the + // bootstrap procedure and the steady state disagree about what a finished + // change looks like. + w("## Finishing a change\n\n") + w("Bootstrap set these up. They are only worth having if every change maintains them.\n\n") + w("1. **Run the gates above, in order**, and say what each reported — not \"it builds\".\n") + w("2. **Capture requirements as they arrive** — " + bt + "./mxcli brain capture \"\"\n") + w(" --slice -a @Mod.Thing" + bt + ". Never tick anything off: " + bt + "brain plan" + bt + " derives what\n") + w(" is built from the model, so finishing the work is what moves the number.\n") + w("3. **Append to " + bt + "FINDINGS.md" + bt + "** — anything surprising or broken, and how you verified it.\n") + w("4. **Commit.** An idle session is reaped and its container goes with it; whatever is\n") + w(" uncommitted is gone, including the model.\n\n") + return sb.String() } diff --git a/cmd/mxcli/init_claudemd_gates_test.go b/cmd/mxcli/init_claudemd_gates_test.go new file mode 100644 index 0000000000..850f45db85 --- /dev/null +++ b/cmd/mxcli/init_claudemd_gates_test.go @@ -0,0 +1,119 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "strings" + "testing" +) + +// The gate list is stated in three places, each of which is the only one some +// reader ever sees: +// +// - the generated CLAUDE.md, re-read into EVERY session in the project, so it +// is what an agent does by default without being asked; +// - the `bootstrap-app` skill, which provisions the project and hands over; +// - docs-site's bootstrap-prompt page, which is what a human reads before +// pasting the seed prompt. +// +// They drifted, and the drift was invisible: `mxcli test` appeared in the +// skill only as a ports aside ("avoid 8081/8091/6544") and on no gate list at +// all, so testing was reachable only by a user asking for it by name — while +// `check` was named in 41 skills and ran almost every time. A gate stated in +// two places out of three is a gate that runs when someone remembers. +// +// These tests hold the three to projectGates, which is the one list. + +// bootstrapSkill returns the bootstrap-app SKILL.md as it SHIPS (from the +// embedded tree), not as it sits in .claude/. The embedded copy is what a user +// gets, and reading it also catches a skill edit that was never synced. +func bootstrapSkill(t *testing.T) string { + t.Helper() + const embedded = "skills/bootstrap-app/SKILL.md" + b, err := skillsFS.ReadFile(embedded) + if err != nil { + t.Fatalf("cannot read embedded %s: %v\nRun `make sync-skills` (or `make build`) to mirror "+ + ".claude/skills/mendix/ into cmd/mxcli/skills/.", embedded, err) + } + return string(b) +} + +func TestEveryGateIsARealCommand(t *testing.T) { + for _, g := range projectGates { + if _, _, err := rootCmd.Find(strings.Fields(g.Command)); err != nil { + t.Errorf("gate %q names `mxcli %s`, which is not a registered command: %v", + g.Ref, g.Command, err) + } + } +} + +// The generated CLAUDE.md is the default behaviour: a gate missing here is a +// gate an agent never runs unless the user names it. +func TestGeneratedClaudeMDPublishesEveryGate(t *testing.T) { + md := generateClaudeMD("Demo", "Demo.mpr") + for _, g := range projectGates { + if !strings.Contains(md, "./"+g.Ref) { + t.Errorf("generated CLAUDE.md does not name the %q gate; it is then run only when "+ + "the user asks for it by name", g.Ref) + } + } +} + +// The skill provisions the project and is the last thing read before the work +// starts, so it has to name the same gates the project's CLAUDE.md will. +func TestBootstrapSkillNamesEveryGate(t *testing.T) { + skill := bootstrapSkill(t) + for _, g := range projectGates { + if !strings.Contains(skill, g.Ref) { + t.Errorf("the bootstrap-app skill does not name the %q gate, but the CLAUDE.md it "+ + "provisions does. Update .claude/skills/mendix/bootstrap-app/SKILL.md and re-run "+ + "`make sync-skills`.", g.Ref) + } + } +} + +// The docs page describes the skill to a human deciding whether to trust it. A +// step the page omits is a step nobody reviews. +func TestBootstrapPromptDocNamesEveryGate(t *testing.T) { + const path = "../../docs-site/src/tools/bootstrap-prompt.md" + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("cannot read %s: %v", path, err) + } + doc := string(b) + for _, g := range projectGates { + if !strings.Contains(doc, g.Ref) { + t.Errorf("%s does not name the %q gate, but the procedure it documents runs it", path, g.Ref) + } + } +} + +// The three descriptions of the procedure also have to agree about the quality +// report and the plan, which are the parts a coding agent skips first: they +// produce no error when omitted, so nothing else notices. +func TestBootstrapAndDefaultBehaviourAgreeOnQualityAndPlan(t *testing.T) { + md := generateClaudeMD("Demo", "Demo.mpr") + skill := bootstrapSkill(t) + + for _, want := range []struct{ substr, why string }{ + {"brain capture", "capturing requirements as they arrive is what keeps `brain plan` a real progress report"}, + {"FINDINGS.md", "the bootstrap creates it; without a rule to append to it, it is written once and then stops being true"}, + } { + if !strings.Contains(md, want.substr) { + t.Errorf("generated CLAUDE.md does not mention %q — %s", want.substr, want.why) + } + if !strings.Contains(skill, want.substr) { + t.Errorf("bootstrap-app skill does not mention %q — %s", want.substr, want.why) + } + } + + // The baseline is only meaningful taken before any of the user's own work + // is in the project, so the skill has to run `report` during provisioning, + // not merely list it as something that exists. + if !strings.Contains(skill, "quality baseline") { + t.Error("bootstrap-app skill no longer takes a quality baseline; the `report` scores it " + + "leaves behind are then uncomparable, because every later figure is the user's work " + + "plus the template's with nothing to subtract") + } +} diff --git a/cmd/mxcli/setup.go b/cmd/mxcli/setup.go index f286067a79..2f0788a4cd 100644 --- a/cmd/mxcli/setup.go +++ b/cmd/mxcli/setup.go @@ -76,6 +76,26 @@ Examples: _ = reader.Disconnect() versionStr = pv.ProductVersion fmt.Fprintf(os.Stdout, "Detected Mendix version: %s\n", versionStr) + } else { + // A hand-typed version may name no CDN artifact: Mendix 9 and 10 + // publish a build number the release notes never mention, so + // "10.24.25" is really mxbuild-10.24.25.122571.tar.gz. Resolving it + // here turns a bare 404 into the download the user asked for. A + // project-detected version needs none of this — _ProductVersion + // already carries all four parts — so this is the else branch. + resolved, err := docker.ResolveCDNVersion(versionStr, runtime.GOARCH) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + if avail := docker.CDNReleasesFor(majorMinorOf(versionStr), runtime.GOARCH); len(avail) > 0 { + fmt.Fprintf(os.Stderr, " Published for %s: %s\n", + majorMinorOf(versionStr), strings.Join(avail[:minInt(len(avail), 8)], ", ")) + } + os.Exit(1) + } + if resolved != versionStr { + fmt.Fprintf(os.Stdout, "Resolved Mendix %s to %s\n", versionStr, resolved) + versionStr = resolved + } } // The Mendix CDN only publishes Linux mxbuild. On Windows/macOS a CDN @@ -325,3 +345,21 @@ func init() { setupCmd.AddCommand(setupMxcliCmd) rootCmd.AddCommand(setupCmd) } + +// majorMinorOf reduces a version to "major.minor" for listing what the CDN +// publishes near a version that could not be resolved. A version with no minor +// is returned as-is, which still narrows the listing to one major. +func majorMinorOf(version string) string { + parts := strings.Split(version, ".") + if len(parts) >= 2 { + return parts[0] + "." + parts[1] + } + return version +} + +func minInt(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/docs-site/src/appendixes/version-compatibility.md b/docs-site/src/appendixes/version-compatibility.md index b8d0ca2252..fa66f8a6cb 100644 --- a/docs-site/src/appendixes/version-compatibility.md +++ b/docs-site/src/appendixes/version-compatibility.md @@ -76,9 +76,9 @@ The tables below show exactly which features are available on each Mendix versio | Conditional visibility | `Visible: [xpath]` | -- | -- | -- | Yes | | Conditional editability | `Editable: [xpath]` | -- | -- | -- | Yes | | Responsive column widths | `TabletWidth: 6, PhoneWidth: 12` | -- | -- | -- | Yes | -| Page parameters (entity) | `Params: { $Item: Module.Entity }` | -- | -- | -- | Yes | +| Page parameters (entity) | `Params: { $Item: Module.Entity }` | 9.4+ | Yes | Yes | Yes | | Page parameters (primitive) | `Params: { $Qty: Integer }` | -- | -- | -- | 11.6+ | -| Page variables | `Variables: { ... }` | -- | -- | -- | Yes | +| Page variables | `Variables: { ... }` | -- | 10.20+ | 10.20+ | Yes | | Design properties (Atlas v3) | `DesignProperties: [...]` | -- | -- | -- | Yes | ::: tip Widget Templates @@ -146,9 +146,11 @@ The Mendix metamodel evolves across versions. The reflection data shows ~42% typ View entities exist in both 10.18+ and 11.x, but the BSON structure differs. In Mendix 10.x, the `OqlViewEntitySource` object has an `Oql` field that stores the OQL query inline (in addition to the separate `ViewEntitySourceDocument`). Mendix 11.0 removed the inline `Oql` field. The writer detects the project version and includes the inline field for 10.x projects. -### Page Parameters (10.x vs 11.x) +### Page Parameters (9.4+, with an 11.5 tail) -Mendix 11.0 changed how page parameters are stored in BSON. The `Variable` property in page parameter mappings uses a different structure. Writing 11.x-style page parameters to a 10.x project causes an `InvalidOperationException`. +Page parameters are not an 11.x feature. `Pages$PageParameter` and `Page.parameters` were introduced in **9.4.0**, and `Pages$PageSettings.parameterMappings` — the arguments a `SHOW PAGE` passes — in **9.7.0** (measured from the Mendix Model SDK's `StructureVersionInfo` records). mxcli refused them on 10.x until [mendixlabs/mxcli#1121](https://github.com/mendixlabs/mxcli/issues/1121); the 11.0 floor in the version registry came from an illustrative sample table in a proposal, not from a measurement. + +What *is* version-specific is the optional/default-value half: `PageParameter.IsRequired` and `PageParameter.DefaultValue` arrived in **11.5.0**. MDL cannot express either — every parameter it writes is required with no default — so the writer omits both below 11.5. Emitting them anyway is the "never invent a key" failure: mxbuild accepts unknown properties, and Studio Pro throws `InvalidOperationException` at `MprProperty.cs`. ### Design Properties (Atlas v2 vs v3) diff --git a/docs-site/src/internals/catalog-schema.md b/docs-site/src/internals/catalog-schema.md index 30fa0eed95..c6771e8f38 100644 --- a/docs-site/src/internals/catalog-schema.md +++ b/docs-site/src/internals/catalog-schema.md @@ -197,9 +197,21 @@ rather than trusting a list here: | `home_page` / `login_page` / `menu_item` | navigation profile references a page | | `calculate` | calculated attribute uses a microflow | | `schedule` | scheduled event runs a microflow | +| `publish` | published REST operation runs a microflow | +| `event` | entity event handler runs a microflow | +| `settings` | a project setting (after-startup / before-shutdown / health check) names a microflow | +| `sync` | offline navigation profile synchronizes an entity | | `validate` | attribute validation rule uses a regular expression | | `widget` | page or snippet uses a pluggable / custom widget | +`schedule`, `publish`, `event` and `settings` are **entry points**: something +outside the call graph runs the microflow, so nothing in the model calls it. +They are what stops `GRAPH_DEAD_ASSETS`, `SHOW CALLERS OF` and lint rule QUAL004 +from reporting a live scheduled job, API handler or event handler as unused. +`sync` is not one — it names an entity a profile downloads, which is a use of a +type, so `SHOW CALLERS` excludes it for the same reason it excludes +`datasource`. + #### WIDGET targets A `widget` edge is the odd one out and is worth knowing about before you join diff --git a/docs-site/src/reference/capabilities.md b/docs-site/src/reference/capabilities.md index 6b46b2df6d..c6e8374148 100644 --- a/docs-site/src/reference/capabilities.md +++ b/docs-site/src/reference/capabilities.md @@ -48,7 +48,7 @@ Everything mxcli can do, organized by use case. | Retrieve | `RETRIEVE ... FROM ... WHERE` | Database/association queries | | Control flow | `IF/THEN/ELSE`, `LOOP`, `WHILE` | Including nested | | Call flows | `CALL MICROFLOW`, `CALL NANOFLOW` | With parameters | -| Show page | `LIST PAGE Module.Page(...)` | With page parameters (11.0+) | +| Show page | `LIST PAGE Module.Page(...)` | With page parameters (9.7+) | | REST requests | `SEND REST REQUEST` | GET/POST/PUT/DELETE | | Database queries | `EXECUTE DATABASE QUERY` | External databases | | Log messages | `LOG INFO\|WARNING\|ERROR` | With templates | @@ -140,7 +140,6 @@ Everything mxcli can do, organized by use case. | Area | Limitation | Workaround | |---|---|---| -| Page parameters | Requires Mendix 11.0+ | Use non-persistent entity pattern on 10.x | | Design properties (Atlas v3) | Requires Mendix 11.0+ | Use CSS classes on 10.x | | REST query parameters | Requires Mendix 11.0+ | Build query string manually on 10.x | | Pluggable widget ImageUrl mode | Cannot set imageUrl from MDL | Configure in Studio Pro | diff --git a/docs-site/src/tools/bootstrap-prompt.md b/docs-site/src/tools/bootstrap-prompt.md index d537889744..d61ffd29d8 100644 --- a/docs-site/src/tools/bootstrap-prompt.md +++ b/docs-site/src/tools/bootstrap-prompt.md @@ -75,7 +75,12 @@ a longer prompt. is no status column to maintain. 5. **Commits, then boots and verifies** — HTTP 200 at `http://localhost:8080/`, plus an optional `run --hub` preview URL. -6. **Proposes the model in MDL and waits** — module, entities, roles, pages — before +6. **Takes the quality baseline** — `mxcli lint` and the scored `mxcli report` on the + blank app, recorded in `FINDINGS.md`. This is the only moment those numbers mean + "what the template ships with"; afterwards every figure is yours plus the + template's, with nothing to subtract. See [mxcli report](mxcli-report.md) for how to + read the six category scores. +7. **Proposes the model in MDL and waits** — module, entities, roles, pages — before building anything. For a solution repo it also covers the parts that bite: per-app ports, a hostname per @@ -140,7 +145,7 @@ keep straight. ## Two rules that make this robust -- **Committing the config is mandatory** (the skill's provisioning step 6). The prompt +- **Committing the config is mandatory** (the skill's provisioning step 7). The prompt is a *one-time seed*. Its output — `.mpr` + `.devcontainer/` + `.claude/` with the SessionStart hook and `bootstrap-mxcli.sh` — must be committed so the steady state is file-driven and deterministic. After that, every new session runs the hook @@ -161,6 +166,35 @@ keep straight. ./mxcli exec change.mdl -p .mpr # edit the model; the loop hot-applies ``` +### The gates + +`mxcli init` writes this list into the project's `CLAUDE.md`, so an agent has it in +context in every session without being asked — it is the **definition of done**, not a +menu. The same list is in the `bootstrap-app` skill, and the three are held together +by a test, because a gate that is named in two of the three places is a gate that only +runs when someone remembers to ask for it. + +```bash +./mxcli check change.mdl -p .mpr --references # syntax + references (~2s) +./mxcli exec change.mdl -p .mpr # apply +./mxcli lint -p .mpr # rules (~3s) +./mxcli report -p .mpr # scored quality report +./mxcli docker check -p .mpr # mxbuild, the slow one (~25s) +./mxcli test tests/ -p .mpr --local # microflow tests (~30s cold, ~2s warm) +./mxcli run --local --watch -p .mpr # the app, hot-reloading +``` + +Each buys something the one above it cannot: [`lint`](mxcli-lint.md) and +[`report`](mxcli-report.md) score the model that `check` only proved was well-formed, +[`test`](testing.md) proves behaviour that a clean build does not, and the running app +is the only thing that proves how a page renders. `report`'s six category scores are +comparable against the baseline taken at bootstrap — a score that fell is a finding. + +Writing the first test with the first microflow, rather than later, is what keeps +`test` from being permanently skipped: after the code is written the expected values +have stopped being obvious. `--local` needs no Docker daemon, so it works in a web +session. + In a solution, run one loop per app from its own folder, with the second app on the alternate ports, and start the producer first so the consumer's external entities resolve: diff --git a/docs-site/src/tools/catalog-tables.md b/docs-site/src/tools/catalog-tables.md index a2c450926e..50ba20cfc6 100644 --- a/docs-site/src/tools/catalog-tables.md +++ b/docs-site/src/tools/catalog-tables.md @@ -243,6 +243,46 @@ profile with `sync Sales.Audit never` still *names* that entity, so renaming or dropping it leaves the configuration dangling — which is precisely what a reference edge exists to reveal. +### Entity event handlers + +`CATALOG.ENTITY_EVENT_HANDLERS` — one row per entity event handler: which +moment, which event, and which microflow runs. + +```sql +select EntityQualifiedName, Moment, Event, Microflow + from CATALOG.ENTITY_EVENT_HANDLERS + where Moment = 'Before' and Event = 'Commit'; +``` + +`CATALOG.ENTITIES.HasEventHandlers` says some exist and nothing else — the same +shape `NavigationProfile.OfflineEntityCount` had. The distinction the flag loses +is the one that matters: a `Before` handler with `RaiseErrorOnFalse` can **veto** +the commit, an `After` handler cannot. + +`Moment` is `Before` or `After`. `Event` is the value Mendix stores, not the +caption Studio Pro shows: `Create`, `Commit`, `Delete`, `RollBack` — note the +**capital B**, which `generated/metamodel` confirms +(`DomainModelsEventRollBack = "RollBack"`) and which disagrees with every +neighbouring enum in that file, where the same word is `Rollback`. A query +spelling it the expected way returns zero rows rather than an error. + +A handler also produces an `event` row in `CATALOG.REFS`, so a microflow that +runs only as a handler has callers: + +```sql +select SourceName, TargetName from CATALOG.REFS where RefKind = 'event'; +``` + +Without that edge the handler microflow was reported dead from three directions +at once — `show callers` said `(no callers found)`, `CATALOG.GRAPH_DEAD_ASSETS` +listed it, and `mxcli lint` emitted **QUAL004** "is not called from anywhere" +with the suggestion *Remove if unused* — on code that runs on every commit +([mendixlabs/mxcli#1127](https://github.com/mendixlabs/mxcli/issues/1127)). + +The edge carries neither the moment nor the event; `refs` has no column for +them, and a kind per combination would put eight kinds into every consumer's +list to say one thing. Which moment and which event is this table's question. + ## Graph-Analysis Tables The dependency graph (`CATALOG.REFS`, full refresh) is analysed by a family of diff --git a/docs-site/src/tools/run-local.md b/docs-site/src/tools/run-local.md index 5977fa1515..6ed2c883c4 100644 --- a/docs-site/src/tools/run-local.md +++ b/docs-site/src/tools/run-local.md @@ -77,6 +77,7 @@ so structural changes need a restart; behavioural changes do not. | `--app-port` | 8080 | App HTTP port | | `--admin-port` | 8090 | M2EE admin API port | | `--serve-port` | 6543 | `mxbuild --serve` port | +| `--mxbuild-path` | resolved for this host | The mxbuild to build with, overriding resolution (Studio Pro on macOS/Windows, the cached CDN download on Linux) | | `--db-host` | 127.0.0.1:5432 | Database `host:port`; bracket IPv6 endpoints (`[::1]:5432`) | | `--db-name` | derived from project | Database name | | `--db-user` / `--db-password` | mendix / mendix | Database credentials | @@ -379,6 +380,16 @@ not just headless checks. no bundler to keep hot — `run --local` prints a line saying so and skips it. Both paths work; the bundle is mxbuild's rather than mxcli's. +**Classic-client apps need no bundling at all.** With **Web UI Settings > +OptimizedClient = No**, mxbuild puts Mendix's classic (Dojo) client in +`deployment/web/` — loaded from `mxclientsystem` and served as-is — and parks the +React client in `deployment/react-web/`. (With `Yes` it is the other way round: +React in `web/`, Dojo in `dojo-web/`.) There is no rollup step and no `web/dist`, +so `run --local` says so and skips it, under `--watch` too. mxcli reads this from +the deployment's own `web/index.html` rather than from the model setting, since +the deployment is what gets served and the two disagree right after the setting +is changed. + ### `--watch` on Mendix 11.14 **`--watch` is not usable on 11.14 yet.** It starts and the app boots, but every diff --git a/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md b/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md index 2043c516b2..1e66788bbf 100644 --- a/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md +++ b/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md @@ -98,6 +98,37 @@ user changed by hand with values derived from somewhere else. A field set on the construct the element separately, so both need checking, by grepping the struct literal rather than the field name. +**A guard at one layer is not a guard for the property.** A round-trip test that +proves the codec reads a property back says nothing about whether the caller +above it overwrites the value first. Two execution flags had exactly such a test, +green since the day their codec bug was fixed, while the executor's rebuild +struct kept stamping literals over both before the codec ever saw them. When a +property is being reset, find the **last** writer on the path, not the first one +that looks responsible. + +**Which way the reset goes decides whether anything catches it.** The same two +flags were lost twice in opposite directions. The codec wrote Go's zero value, +turning *allow concurrent execution* into *disallow* — and disallow without an +error message is CE4899, so it was caught at once. The executor's literal wrote +the opposite, turning *disallow* into *allow* — and CE4899 never fires on allow, +so the reset silently switched off the one error that covers this area. A checker +that catches a property's loss in one direction is not coverage for that +property; ask what the *other* direction produces. + +**Order the candidates by what makes them findable, not by severity.** A +mechanical audit produces the candidate list; it does not say which candidate +gets found before a user hits it. Two things do that, and neither is severity. A +property is findable if some gate downstream complains — the concurrency flags +became CE4899, a cleared exclusion became CE0122 — or if it is *salient* enough +that someone thinks to audit it: "apply entity access" was caught in-house purely +because it is a security setting, with every checker silent. A microflow's +deep-link URL is neither. It breaks no build, it narrows no permission, nothing +in `describe` shows it missing, and the only place it exists after the rewrite is +Studio Pro's properties pane — so it sat there until a user reported it. The +properties that are neither checked nor interesting are not this class's +low-severity tail; they are the part of it that reaches users, and they are +exactly what an audit ordered by "what could go badly wrong?" leaves for last. + **Partial statements are the honest hazard.** `create or modify entity` with a subset of attributes drops the rest, which is arguably what "modify to this shape" means. The remedy there was not refusal but telling the truth loudly: diff the diff --git a/mdl-examples/bug-tests/1121-page-parameters-on-mendix-10.mdl b/mdl-examples/bug-tests/1121-page-parameters-on-mendix-10.mdl new file mode 100644 index 0000000000..a530dabbd0 --- /dev/null +++ b/mdl-examples/bug-tests/1121-page-parameters-on-mendix-10.mdl @@ -0,0 +1,61 @@ +-- mendixlabs/mxcli#1121 — CREATE PAGE with Params: was refused on every Mendix 10 +-- project, so no parameterised page could be authored from MDL there at all. +-- +-- REPORTED SHAPE (Mendix 10.24.25, MPR v2): +-- +-- create page with parameters requires Mendix 11.0+ (project is 10.24.25) +-- hint: pass data via a non-persistent entity or microflow parameter instead +-- +-- The 11.0 floor was never measured. It matches the illustrative `show features` +-- sample table in docs/11-proposals/PROPOSAL_version_aware_agent_support.md and +-- nothing else. Measured against the Mendix Model SDK's own StructureVersionInfo +-- records (mendixmodelsdk 4.115.0, src/gen/pages.js): +-- +-- Pages$PageParameter, Page.parameters 9.4.0 +-- Pages$PageSettings.parameterMappings 9.7.0 (SHOW PAGE arguments) +-- Pages$PageParameter.isRequired 11.5.0 +-- Pages$PageParameter.defaultValue 11.5.0 +-- +-- So the ELEMENT is 9.4 and only its optional/default-value half is 11.5. The +-- fix splits the two: the registry floor drops to 9.4, and the writer emits +-- IsRequired/DefaultValue only on 11.5+ — MDL cannot express either, so below +-- 11.5 they are two keys the project's metamodel does not declare. mxbuild +-- accepts unknown properties; Studio Pro throws InvalidOperationException at +-- MprProperty.cs, so lifting the gate without that guard would have traded an +-- honest refusal for a page Studio Pro cannot open. +-- +-- 11.0–11.4 is the row that shows the guard is not cosmetic: those versions +-- PASSED the old 11.0 gate and got both 11.5-only keys written. +-- +-- Run against a Mendix 10.x project; before the fix, statement 2 is refused. + +CREATE MODULE Bug1121; + +CREATE PERSISTENT ENTITY Bug1121.Item ( + Name: String(200) +); + +-- 1. A page that takes an object parameter and binds a data view to it. +CREATE OR REPLACE PAGE Bug1121.Item_Edit ( + Title: 'Edit item', + Layout: Atlas_Core.PopupLayout, + Params: { $Item: Bug1121.Item } +) { + layoutgrid lg { + row r { + column c (DesktopWidth: AutoFill) { + dataview dv (DataSource: $Item) { + textbox txtName (Label: 'Name', Attribute: Name) + } + } + } + } +} + +-- 2. Passing the argument from a microflow. Without this half a parameterised +-- page is unreachable, which is the other thing the report calls out. +CREATE OR REPLACE MICROFLOW Bug1121.ACT_EditItem ( + $Item: Bug1121.Item +) BEGIN + SHOW PAGE Bug1121.Item_Edit ($Item = $Item); +END diff --git a/mdl-examples/bug-tests/catalog-1127-entity-event-handler-refs.mdl b/mdl-examples/bug-tests/catalog-1127-entity-event-handler-refs.mdl new file mode 100644 index 0000000000..7edad40b85 --- /dev/null +++ b/mdl-examples/bug-tests/catalog-1127-entity-event-handler-refs.mdl @@ -0,0 +1,74 @@ +-- ============================================================================ +-- Issue mendixlabs/mxcli#1127 — entity event handler microflows read as dead +-- ============================================================================ +-- +-- The file is named with a topic prefix rather than a bare `1127-`: the +-- historical files here are named after mendixlabs/mxcli PR numbers, and the +-- fork's issue numbers already collide with them on 261-266. The reference is +-- written qualified everywhere for the same reason. +-- +-- An entity event handler runs a microflow on every create/commit/delete of its +-- entity. The catalog reduced the whole list to +-- CATALOG.ENTITIES.HasEventHandlers — a flag saying some exist and nothing else +-- — and emitted no `refs` row at all, so every consumer of the reference graph +-- agreed the handler microflow was unreachable: +-- +-- show callers of Mod.ACT_Order_Validate → (no callers found) +-- CATALOG.GRAPH_DEAD_ASSETS → lists it +-- mxcli lint → [QUAL004] "... is not called +-- from anywhere. Remove if unused" +-- +-- Measured on the v1 test project, one variable per run (the extractor stubbed +-- to emit nothing is the control): +-- +-- control (no `event` edge) → 2 dead microflows, "(no callers found)" +-- after the fix → 0 dead microflows, caller = Mod.EvOrder +-- +-- Same class as the scheduled-event gap and worse in degree: a handler runs on +-- every commit, so the false "dead" verdict lands on code hotter than most of +-- what IS reported as live — and the lint suggestion is "Remove if unused". +-- +-- ---------------------------------------------------------------------------- +-- Running this example +-- +-- mxcli exec catalog-1127-entity-event-handler-refs.mdl -p app.mpr +-- mxcli -p app.mpr -c "show callers of Sample.ACT_EvOrder_Validate" +-- # Sample.EvOrder, depth 1 — the entity is the caller +-- mxcli -p app.mpr -c "select EntityQualifiedName, Moment, Event, Microflow, +-- RaiseErrorOnFalse from CATALOG.ENTITY_EVENT_HANDLERS" +-- # two rows: Before/Commit (RaiseErrorOnFalse 1) and After/Commit (0) +-- mxcli -p app.mpr -c "select QualifiedName from CATALOG.GRAPH_DEAD_ASSETS +-- where ObjectType = 'MICROFLOW'" +-- # neither handler is listed +-- +-- The moment and the event live in CATALOG.ENTITY_EVENT_HANDLERS, not in the +-- edge: `refs` has no column for them, and a kind per combination would put +-- eight kinds into every consumer's list to say one thing. The distinction is +-- not cosmetic — a Before handler with `raise error` can veto the commit, an +-- After handler cannot. +-- ============================================================================ + +create entity Sample.EvOrder ( + "Amount": decimal +); +/ + +create microflow Sample.ACT_EvOrder_Validate ($Order: Sample.EvOrder) +returns boolean +begin + return true; +end; +/ + +create microflow Sample.ACT_EvOrder_Notify ($Order: Sample.EvOrder) +begin + log info 'committed'; +end; +/ + +-- The Before handler can veto the commit; the After one only observes it. +alter entity Sample.EvOrder + add event handler on before commit call Sample.ACT_EvOrder_Validate($currentObject) raise error; + +alter entity Sample.EvOrder + add event handler on after commit call Sample.ACT_EvOrder_Notify($currentObject); diff --git a/mdl/backend/modelsdk/microflow.go b/mdl/backend/modelsdk/microflow.go index f3fe7fc0ab..9ae6798dad 100644 --- a/mdl/backend/modelsdk/microflow.go +++ b/mdl/backend/modelsdk/microflow.go @@ -9,6 +9,7 @@ import ( "github.com/mendixlabs/mxcli/modelsdk/element" genDT "github.com/mendixlabs/mxcli/modelsdk/gen/datatypes" genMf "github.com/mendixlabs/mxcli/modelsdk/gen/microflows" + genTexts "github.com/mendixlabs/mxcli/modelsdk/gen/texts" "github.com/mendixlabs/mxcli/modelsdk/mprread" "github.com/mendixlabs/mxcli/model" @@ -205,6 +206,24 @@ func microflowFromGen(mf *genMf.Microflow, containerID model.ID) *microflows.Mic // microflow may read and write. mx check and mxbuild are both silent, // because the model is valid either way. ApplyEntityAccess: mf.ApplyEntityAccess(), + // The two flags above are only half of it: what Mendix does to the + // second caller when concurrency is disallowed lives here, and the + // writer emitted an empty message and no microflow on every rewrite, so + // the pair vanished — translations and all — the moment anything edited + // the microflow. Mendix requires one of them (CE4899). + ConcurrencyErrorMessage: concurrencyMessageFromGen(mf.ConcurrencyErrorMessage()), + ConcurrencyErrorMicroflow: mf.ConcurrencyErrorMicroflowQualifiedName(), + // Studio Pro's "Export level". The writer pinned it to "Hidden", so a + // microflow a protected module exposes as API was demoted to hidden by + // any rewrite — again with every checker silent. + ExportLevel: mf.ExportLevel(), + // The deep link (Mendix 10.6+). Same class again: the writer emitted an + // empty Url on every rewrite and nothing read the stored one back, so a + // CREATE OR MODIFY that touched only the body deleted it. Both checkers + // stay silent — a microflow with no URL is valid — so the loss only + // showed up in Studio Pro (#1120). + URL: mf.Url(), + URLSearchParameters: mf.UrlSearchParametersQualifiedNames(), } out.ID = model.ID(mf.ID()) // AllowedModuleRoles (BY_NAME role references) — without these DESCRIBE omits @@ -231,6 +250,26 @@ func microflowFromGen(mf *genMf.Microflow, containerID model.ID) *microflows.Mic return out } +// concurrencyMessageFromGen reads a microflow's concurrency error message. +// +// nil for a message with no translations, so that a microflow which never had +// one is unchanged by a rewrite: textToGen of an empty model.Text and the bare +// genTexts.NewText() the writer used to emit produce the same document, and +// keeping the distinction out of the model keeps elision (ADR-0008) simple. +// "The same document" is canon.Equal, not bytes.Equal — the element's $ID is +// minted fresh on every encode, which is why canon compares a canonical form. +func concurrencyMessageFromGen(el element.Element) *model.Text { + txt, ok := el.(*genTexts.Text) + if !ok || txt == nil { + return nil + } + out := textFromGen(txt) + if len(out.Translations) == 0 { + return nil + } + return out +} + // flowsFromGen reconstructs the sequence-flow edges (origin/destination + branch // case) from a gen flow list, so DESCRIBE can order activities by the flow graph. func flowsFromGen(items []element.Element) []*microflows.SequenceFlow { diff --git a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go index fc645ced3d..cfd8e7a4b1 100644 --- a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go +++ b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/modelsdk/canon" "github.com/mendixlabs/mxcli/modelsdk/codec" genMf "github.com/mendixlabs/mxcli/modelsdk/gen/microflows" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -87,3 +88,184 @@ func TestMicroflowRoundTrip_ApplyEntityAccess(t *testing.T) { t.Error("ApplyEntityAccess invented on round-trip (want false)") } } + +// TestMicroflowRoundTrip_DeepLinkURL is the fifth property in this struct to go +// the way #723 §A describes, and the first with no checker behind it at all. +// +// A microflow's Url is its deep link (Mendix 10.6+) — Studio Pro's "URL" field, +// e.g. `item/{Key}`. MDL has no syntax for one, so microflowToGen wrote `""` +// unconditionally and microflowFromGen never read the stored value back: a +// CREATE OR MODIFY MICROFLOW that changed only the body deleted the deep link. +// +// Nothing reports it. Unlike the concurrency flags above (CE4899), a microflow +// without a URL is entirely valid, so `mxcli check`, `mx check` and mxbuild all +// pass before and after — the loss is visible only in Studio Pro, which is how +// it reached a user as #1120. UrlSearchParameters is stored beside it and was +// lost with it. +func TestMicroflowRoundTrip_DeepLinkURL(t *testing.T) { + // The two parameters are deliberately DIFFERENT. A parameter used in the + // URL path may not also be a search parameter — mxbuild 11.6.6 rejects that + // with CE5612 ("cannot be used as a URL parameter if it is already a URL + // search parameter"). The first version of this fixture reused `Key` for + // both and described a document Mendix refuses to build; the unit tests + // could not tell, because nothing here validates the model. Measured by + // seeding a real project and running mx check. + mf := µflows.Microflow{ + Name: "ACT_Item", + URL: "item/{Key}", + URLSearchParameters: []string{"Mod.ACT_Item.Filter"}, + } + mf.ID = model.ID("mf-4") + + got := roundTripMicroflow(t, mf) + if got.URL != "item/{Key}" { + t.Errorf("deep-link URL lost on round-trip: got %q, want %q", got.URL, "item/{Key}") + } + if len(got.URLSearchParameters) != 1 || got.URLSearchParameters[0] != "Mod.ACT_Item.Filter" { + t.Errorf("UrlSearchParameters lost on round-trip: got %v, want [Mod.ACT_Item.Filter]", + got.URLSearchParameters) + } + + // The other direction: a microflow that has no deep link must not acquire + // one, or the fix is a different silent change in the same place. An empty + // UrlSearchParameters must stay the empty marker-1 list the codec writes. + none := µflows.Microflow{Name: "ACT_Plain"} + none.ID = model.ID("mf-5") + if got := roundTripMicroflow(t, none); got.URL != "" || len(got.URLSearchParameters) != 0 { + t.Errorf("deep link invented on round-trip: URL=%q params=%v", got.URL, got.URLSearchParameters) + } +} + +// TestMicroflowRoundTrip_ExportLevel is the #1120 sibling found by the audit +// the fix prompted: grepping microflowToGen for the constants it writes turned +// up `SetExportLevel("Hidden")` next to `SetUrl("")`. +// +// Export level is Studio Pro's Hidden/API switch — whether the microflow is +// part of the module's public surface when the module is exported as a package. +// Pinning it to Hidden quietly shrinks a protected module's API, and like the +// URL it has no checker behind it: a hidden microflow is a valid microflow. +// +// Measured across three real marketplace modules (Business Events 3.12.0, +// External Database Connector 6.2.3 and 6.3.0): 3 of 3 microflows and 55 of 55 +// documents overall store "Hidden", all three modules exporting at module level +// "Source". So Hidden is the right DEFAULT — the assertion below pins that it +// stays one, rather than becoming the only reachable value again. +func TestMicroflowRoundTrip_ExportLevel(t *testing.T) { + api := µflows.Microflow{Name: "ACT_PublicApi", ExportLevel: "API"} + api.ID = model.ID("mf-6") + if got := roundTripMicroflow(t, api); got.ExportLevel != "API" { + t.Errorf("export level demoted on round-trip: got %q, want %q — the "+ + "microflow has silently left the module's public API", got.ExportLevel, "API") + } + + // The default has to hold in both of its forms. A microflow that says + // nothing must come back "Hidden" — never "", which is not a member of + // MicroflowsExportLevel and is exactly the kind of value that gives a + // document mxbuild accepts and Studio Pro cannot open. + for _, stored := range []string{"", "Hidden"} { + mf := µflows.Microflow{Name: "ACT_Internal", ExportLevel: stored} + mf.ID = model.ID("mf-7") + if got := roundTripMicroflow(t, mf); got.ExportLevel != "Hidden" { + t.Errorf("stored %q came back %q, want %q", stored, got.ExportLevel, "Hidden") + } + } +} + +// TestMicroflowRoundTrip_ConcurrencyErrorHandling covers the other half of the +// concurrency settings: what Mendix does to a second caller when concurrent +// execution is disallowed. Mendix requires one of the two (CE4899), and the +// writer emitted an empty message and no microflow unconditionally, so a +// rewrite deleted the answer — translations included. +func TestMicroflowRoundTrip_ConcurrencyErrorHandling(t *testing.T) { + mf := µflows.Microflow{ + Name: "ACT_Serial", + AllowConcurrentExecution: false, + ConcurrencyErrorMessage: &model.Text{Translations: map[string]string{ + "en_US": "Already running", + "nl_NL": "Wordt al uitgevoerd", + }}, + ConcurrencyErrorMicroflow: "MyModule.ACT_OnBusy", + } + mf.ID = model.ID("mf-8") + + got := roundTripMicroflow(t, mf) + if got.AllowConcurrentExecution { + t.Error("AllowConcurrentExecution flipped to true — the app's concurrency protection is gone") + } + if got.ConcurrencyErrorMicroflow != "MyModule.ACT_OnBusy" { + t.Errorf("ConcurrencyErrorMicroflow = %q, want MyModule.ACT_OnBusy", got.ConcurrencyErrorMicroflow) + } + // Every translation, not just the source language: a message that comes + // back with one of its two languages is the translated-caption loss that + // the delete-then-create path produced elsewhere. + if got.ConcurrencyErrorMessage == nil { + t.Fatal("ConcurrencyErrorMessage lost entirely → CE4899 on a disallow-concurrency microflow") + } + for lang, want := range mf.ConcurrencyErrorMessage.Translations { + if got.ConcurrencyErrorMessage.Translations[lang] != want { + t.Errorf("translation %s = %q, want %q", lang, + got.ConcurrencyErrorMessage.Translations[lang], want) + } + } +} + +// TestMicroflowRoundTrip_NoConcurrencyMessageIsInert is the control that keeps +// the carry from disturbing the ordinary microflow — the one with no +// concurrency message at all, which is nearly all of them. +// +// Before the carry, the writer always emitted a bare empty Texts$Text. A nil +// message must still produce exactly that, or every microflow in every project +// would come out different and defeat write elision (ADR-0008) — a "fix" that +// rewrites the whole project is worse than the bug. +// +// The comparison is canon.Equal on the MESSAGE ELEMENT, and both halves of that +// are deliberate. Not bytes: every encode mints a fresh random $ID per +// sub-element, which is the reason canon compares a canonical form at all. Not +// the whole microflow either: canon.Equal does not mask, and a microflow's +// StableId is a fresh GUID *value* on every encode, so two encodes of one +// unchanged microflow are never Equal — only Reconcile, which masks the +// identity fields, may be asked that question. +func TestMicroflowRoundTrip_NoConcurrencyMessageIsInert(t *testing.T) { + encMessage := func(t *testing.T, mf *microflows.Microflow) []byte { + t.Helper() + mf.ID = model.ID("mf-9") + raw, err := (&codec.Encoder{}).Encode(microflowToGen(mf, 11).ConcurrencyErrorMessage()) + if err != nil { + t.Fatalf("encode: %v", err) + } + return raw + } + + absent := encMessage(t, µflows.Microflow{Name: "ACT_Plain", AllowConcurrentExecution: true}) + empty := encMessage(t, µflows.Microflow{ + Name: "ACT_Plain", AllowConcurrentExecution: true, + ConcurrencyErrorMessage: &model.Text{Translations: map[string]string{}}, + }) + same, err := canon.Equal(absent, empty) + if err != nil { + t.Fatalf("canon.Equal: %v", err) + } + if !same { + t.Error("a nil concurrency message no longer encodes as the bare empty Texts$Text " + + "the writer always wrote; every microflow in every project would be rewritten " + + "on the next run (ADR-0008 elision)") + } + + // The control for the control: canon.Equal must still be able to tell an + // empty message from a real one, or the assertion above proves nothing. + real := encMessage(t, µflows.Microflow{ + Name: "ACT_Plain", AllowConcurrentExecution: true, + ConcurrencyErrorMessage: &model.Text{Translations: map[string]string{"en_US": "Busy"}}, + }) + if same, _ := canon.Equal(absent, real); same { + t.Error("canon.Equal cannot see a real concurrency message") + } + + // And the model must not invent one on the way back. + plain := µflows.Microflow{Name: "ACT_Plain", AllowConcurrentExecution: true} + plain.ID = model.ID("mf-9") + if got := roundTripMicroflow(t, plain); got.ConcurrencyErrorMessage != nil { + t.Errorf("an absent concurrency message came back as %#v, want nil", + got.ConcurrencyErrorMessage) + } +} diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 47b61ede7d..117552c6aa 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -203,14 +203,35 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { out.SetName(mf.Name) out.SetDocumentation(mf.Documentation) out.SetExcluded(mf.Excluded) - out.SetExportLevel("Hidden") + // Carried, not hardcoded — but defaulted, because "" is not a member of + // MicroflowsExportLevel and an enum-valued property must never be written + // with a value the metamodel does not declare. A fresh microflow and a + // stored document that says nothing both get "Hidden", which is what this + // line always wrote, so the default case is unchanged. + exportLevel := mf.ExportLevel + if exportLevel == "" { + exportLevel = "Hidden" + } + out.SetExportLevel(exportLevel) out.SetAllowConcurrentExecution(mf.AllowConcurrentExecution) // Carried, not hardcoded. This was `false` unconditionally, which silently // turned a microflow's "apply entity access" OFF on every rewrite. out.SetApplyEntityAccess(mf.ApplyEntityAccess) out.SetMarkAsUsed(mf.MarkAsUsed) - out.SetConcurrencyErrorMicroflowQualifiedName("") - out.SetConcurrencyErrorMessage(genTexts.NewText()) // empty Texts$Text (Items=[3] via default) + // Carried, not hardcoded. These two are what Mendix does to a second caller + // when AllowConcurrentExecution is false, and Mendix requires one of them in + // that case (CE4899) — writing both empty on every rewrite deleted the + // answer along with the question. nil still yields the bare empty + // Texts$Text this line always wrote (Items=[3] via the registered default), + // so a microflow without a message is unchanged — semantically, which is + // the level that matters: the element's $ID is minted fresh on every encode, + // so it is canon.Equal that holds here, never bytes.Equal (ADR-0008). + out.SetConcurrencyErrorMicroflowQualifiedName(mf.ConcurrencyErrorMicroflow) + if mf.ConcurrencyErrorMessage != nil { + out.SetConcurrencyErrorMessage(textToGen(mf.ConcurrencyErrorMessage)) + } else { + out.SetConcurrencyErrorMessage(genTexts.NewText()) + } out.SetAllowedModuleRolesQualifiedNames(moduleRoleNames(mf.AllowedModuleRoles)) out.SetMicroflowReturnType(microflowDataTypeToGen(mf.ReturnType)) @@ -241,10 +262,14 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { if major >= 10 { out.SetReturnVariableName(mf.ReturnVariableName) - out.SetUrl("") + // Carried, not hardcoded. These two were `""` and `nil` unconditionally, + // so every rewrite deleted the microflow's deep link (#1120). A fresh + // microflow has neither, so the empty values still come out empty — + // SetUrlSearchParametersQualifiedNames(nil) is the empty marker-1 list. + out.SetUrl(mf.URL) // StableId is emitted as a fresh GUID binary via the registered default // (the gen mistypes it as a string), not set here. - out.SetUrlSearchParametersQualifiedNames(nil) // empty marker-1 list + out.SetUrlSearchParametersQualifiedNames(mf.URLSearchParameters) } return out } diff --git a/mdl/backend/modelsdk/page_parameter_version_test.go b/mdl/backend/modelsdk/page_parameter_version_test.go new file mode 100644 index 0000000000..c4d128b0db --- /dev/null +++ b/mdl/backend/modelsdk/page_parameter_version_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/sdk/pages" + "go.mongodb.org/mongo-driver/bson" +) + +// Forms$PageParameter carries two properties Mendix introduced in 11.5.0: +// IsRequired and DefaultValue (mendixmodelsdk 4.115.0, PageParameter.versionInfo). +// The element itself is 9.4.0. +// +// MDL has no syntax for either — every page parameter is required and has no +// default — so below 11.5 they are not information, they are two keys the +// project's metamodel does not declare. mxbuild tolerates unknown properties; +// Studio Pro resolves every stored property against the type's property list and +// throws System.InvalidOperationException at MprProperty.cs, so a green build is +// not a safety net here (see CLAUDE.md, "Overlay Writes: Never Invent a Key"). +// +// This matters because mendixlabs/mxcli#1121 lifted the (wrong) 11.0 version gate +// that had been hiding it: without this guard, opening page parameters up to 10.x +// would trade an honest refusal for a document Studio Pro cannot open. +func TestPageParameterOmits11_5KeysBelow11_5(t *testing.T) { + has := func(t *testing.T, pv *types.ProjectVersion, key string) bool { + t.Helper() + g := pageParameterToGen(&pages.PageParameter{ + Name: "Item", + EntityName: "MyModule.Item", + IsRequired: true, + }, pv) + b, err := (&codec.Encoder{}).Encode(g) + if err != nil { + t.Fatalf("encode: %v", err) + } + _, err = bson.Raw(b).LookupErr(key) + return err == nil + } + + v := func(major, minor, patch int) *types.ProjectVersion { + return &types.ProjectVersion{MajorVersion: major, MinorVersion: minor, PatchVersion: patch} + } + + tests := []struct { + name string + pv *types.ProjectVersion + want bool // want IsRequired + DefaultValue emitted + }{ + {"10.24.25 (the project in #1121)", v(10, 24, 25), false}, + {"9.4.0 (floor for the element itself)", v(9, 4, 0), false}, + // 11.0–11.4 passed the old 11.0 gate and got both keys written, so the + // bug was live on released 11.x too. This row is the regression control. + {"11.4.0", v(11, 4, 0), false}, + {"11.5.0 (floor for both properties)", v(11, 5, 0), true}, + {"11.13.0", v(11, 13, 0), true}, + // A backend that cannot report a version must not be guessed at. The + // conservative choice is to omit: an absent optional property is filled + // in on load, an unknown one is unopenable. + {"unknown version", nil, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, key := range []string{"IsRequired", "DefaultValue"} { + if got := has(t, tt.pv, key); got != tt.want { + t.Errorf("%s emitted = %v, want %v", key, got, tt.want) + } + } + }) + } +} + +// Whatever the version, the parameter's own identity must survive — a guard that +// drops Name or ParameterType would pass the test above and write a broken page +// (CE5601/CE5606). +func TestPageParameterKeepsNameAndTypeAtEveryVersion(t *testing.T) { + for _, pv := range []*types.ProjectVersion{ + {MajorVersion: 10, MinorVersion: 24, PatchVersion: 25}, + {MajorVersion: 11, MinorVersion: 13, PatchVersion: 0}, + nil, + } { + g := pageParameterToGen(&pages.PageParameter{ + Name: "Item", + EntityName: "MyModule.Item", + IsRequired: true, + }, pv) + b, err := (&codec.Encoder{}).Encode(g) + if err != nil { + t.Fatalf("encode: %v", err) + } + raw := bson.Raw(b) + if name, err := raw.LookupErr("Name"); err != nil || name.StringValue() != "Item" { + t.Errorf("pv=%v: Name = %v (err %v), want \"Item\"", pv, name, err) + } + pt, err := raw.LookupErr("ParameterType") + if err != nil { + t.Fatalf("pv=%v: ParameterType missing", pv) + } + ent, err := pt.Document().LookupErr("Entity") + if err != nil || ent.StringValue() != "MyModule.Item" { + t.Errorf("pv=%v: ParameterType.Entity = %v (err %v), want \"MyModule.Item\"", pv, ent, err) + } + } +} diff --git a/mdl/backend/modelsdk/page_write.go b/mdl/backend/modelsdk/page_write.go index 4523d05eb7..dad3067752 100644 --- a/mdl/backend/modelsdk/page_write.go +++ b/mdl/backend/modelsdk/page_write.go @@ -5,6 +5,7 @@ package modelsdkbackend import ( "fmt" + "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" "github.com/mendixlabs/mxcli/modelsdk/element" @@ -42,7 +43,7 @@ func (b *Backend) CreatePage(page *pages.Page) error { if page.ID == "" { page.ID = model.ID(mmpr.GenerateID()) } - g, err := pageToGen(page) + g, err := pageToGen(page, b.ProjectVersion()) if err != nil { return err } @@ -74,7 +75,7 @@ func (b *Backend) UpdatePage(page *pages.Page) error { if b.writer == nil { return fmt.Errorf("UpdatePage: not connected for writing") } - g, err := pageToGen(page) + g, err := pageToGen(page, b.ProjectVersion()) if err != nil { return err } @@ -109,7 +110,7 @@ func popupDimension(n int) int32 { // pageToGen builds the full gen Page: header, layout call, the widget tree (under // the layout call's form-call arguments), parameters, and variables. -func pageToGen(page *pages.Page) (*genPg.Page, error) { +func pageToGen(page *pages.Page, pv *types.ProjectVersion) (*genPg.Page, error) { out := genPg.NewPage() out.SetName(page.Name) out.SetDocumentation(page.Documentation) @@ -137,7 +138,7 @@ func pageToGen(page *pages.Page) (*genPg.Page, error) { } for _, p := range page.Parameters { - out.AddParameters(pageParameterToGen(p)) + out.AddParameters(pageParameterToGen(p, pv)) } for _, v := range page.Variables { out.AddVariables(localVariableToGen(v)) @@ -221,18 +222,32 @@ func layoutCallToGen(lc *pages.LayoutCall) (*genPg.LayoutCall, error) { return out, nil } +// pageParameterVersion is the Mendix version that introduced PageParameter's +// IsRequired and DefaultValue properties. The element itself is 9.4.0. +const pageParamOptionalMajor, pageParamOptionalMinor = 11, 5 + // pageParameterToGen converts a page parameter, including its ParameterType — an // entity (DataTypes$ObjectType) or a primitive (DataTypes$StringType, …). Without // the type the parameter can't resolve (CE5601/CE5606). -func pageParameterToGen(p *pages.PageParameter) *genPg.PageParameter { +// +// IsRequired and DefaultValue are written only on 11.5+. MDL cannot express +// either — every page parameter it writes is required with no default — so below +// 11.5 they carry no information and are simply two keys the project's metamodel +// does not declare. That is the CLAUDE.md "never invent a key" case: mxbuild +// accepts unknown properties, Studio Pro throws InvalidOperationException at +// MprProperty.cs. An unreadable version omits them, because an absent optional +// property is filled in on load while an unknown one is unopenable. +func pageParameterToGen(p *pages.PageParameter, pv *types.ProjectVersion) *genPg.PageParameter { gp := genPg.NewPageParameter() if p.ID != "" { gp.SetID(element.ID(p.ID)) } assignID(gp) gp.SetName(p.Name) - gp.SetIsRequired(p.IsRequired) - gp.SetDefaultValue(p.DefaultValue) + if pv != nil && pv.IsAtLeast(pageParamOptionalMajor, pageParamOptionalMinor) { + gp.SetIsRequired(p.IsRequired) + gp.SetDefaultValue(p.DefaultValue) + } gp.SetParameterType(pageParamTypeToGen(p)) return gp } diff --git a/mdl/backend/modelsdk/rule_write.go b/mdl/backend/modelsdk/rule_write.go index 204bdadc31..b84a56c1a6 100644 --- a/mdl/backend/modelsdk/rule_write.go +++ b/mdl/backend/modelsdk/rule_write.go @@ -95,9 +95,13 @@ func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { out := genMf.NewRule() out.SetName(rule.Name) out.SetDocumentation(rule.Documentation) - // Both Studio Pro reference rules store ExportLevel "Hidden", and both - // engines already hardcode it for microflows. Omitting it was the one key - // the first authored rule was missing against the reference document. + // Both Studio Pro reference rules store ExportLevel "Hidden". Omitting it + // was the one key the first authored rule was missing against the reference + // document. Unlike a microflow's, this one is NOT carried: a rule is not + // independently callable, so there is nothing for a module to expose, and + // 2 of 2 reference rules agree. If a rule is ever measured storing "API", + // it needs the same carry microflowToGen has — a field on Rule, a read in + // ruleFromGen, and a "" -> "Hidden" default here. out.SetExportLevel("Hidden") out.SetExcluded(rule.Excluded) out.SetMarkAsUsed(rule.MarkAsUsed) diff --git a/mdl/catalog/builder.go b/mdl/catalog/builder.go index 1e8560cb53..c5722d57d0 100644 --- a/mdl/catalog/builder.go +++ b/mdl/catalog/builder.go @@ -102,6 +102,16 @@ type Builder struct { // collected while cataloguing regexes and emitted by buildReferences. regexRuleRefs []regexRuleRef + // Published REST operation → microflow edges, collected while cataloguing + // the services and emitted by buildReferences. Same arrangement as + // scheduledEventRefs above, and for the same reason: the operation is an + // entry point, so nothing in the model calls the microflow it runs. + publishedRestRefs []publishedRestRef + + // Entity → microflow edges from entity event handlers, collected while + // cataloguing the handlers and emitted by buildReferences. + eventHandlerRefs []eventHandlerRef + // Built-in widget definitions supplied by the caller — used to populate // the widget_definitions catalog table alongside project widgets/. builtinWidgetMetas []WidgetDefinitionMeta @@ -393,6 +403,10 @@ func (b *Builder) Build(progress ProgressFunc) error { return fmt.Errorf("failed to build entities: %w", err) } + if err := b.buildEntityEventHandlers(); err != nil { + return fmt.Errorf("failed to build entity event handlers: %w", err) + } + if err := b.buildAssociations(); err != nil { return fmt.Errorf("failed to build associations: %w", err) } diff --git a/mdl/catalog/builder_entity_events.go b/mdl/catalog/builder_entity_events.go new file mode 100644 index 0000000000..1117953376 --- /dev/null +++ b/mdl/catalog/builder_entity_events.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// Catalog rows and reference edges for entity event handlers. +// +// An entity event handler runs a microflow on every create/commit/delete/ +// rollback of its entity, which makes the handler microflow one of the most +// reachable documents in an app — and, before this builder, one of the few with +// no inbound edge at all. CATALOG.ENTITIES.HasEventHandlers reduced the whole +// list to "some exist", the same shape NavigationProfile.OfflineEntityCount had +// before CATALOG.OFFLINE_ENTITY_CONFIGS, so the two halves land together here: +// a row per handler, and an ENTITY -> MICROFLOW edge per handler. + +// eventHandlerRef is one entity → microflow edge for an event handler, held +// until buildReferences (a later pass) emits it. +type eventHandlerRef struct{ entityQualifiedName, moduleName, microflow string } + +// entityEventHandlerRow is one row of entity_event_handlers_data, plus the +// fields the edge needs. Built by a pure function so the mapping — and the +// skips — are testable without a project. +type entityEventHandlerRow struct { + id string + entityID string + entityQualifiedName string + moduleName string + moment string + event string + microflow string + raiseErrorOnFalse bool + passEventObject bool +} + +// entityEventHandlerRows maps one entity's handlers to catalog rows. +// +// A handler naming no microflow is skipped: it is a hole in the document, not a +// reference, and a row for it would put an empty TargetName in refs — which +// matches nothing and counts as an edge in every aggregate. +func entityEventHandlerRows(entity *domainmodel.Entity, moduleName string) []entityEventHandlerRow { + if entity == nil { + return nil + } + entityQN := moduleName + "." + entity.Name + rows := make([]entityEventHandlerRow, 0, len(entity.EventHandlers)) + for _, eh := range entity.EventHandlers { + if eh == nil || eh.MicroflowName == "" { + continue + } + rows = append(rows, entityEventHandlerRow{ + id: string(eh.ID), + entityID: string(entity.ID), + entityQualifiedName: entityQN, + moduleName: moduleName, + moment: string(eh.Moment), + event: string(eh.Event), + microflow: eh.MicroflowName, + raiseErrorOnFalse: eh.RaiseErrorOnFalse, + passEventObject: eh.PassEventObject, + }) + } + return rows +} + +// buildEntityEventHandlers catalogs every entity's event handlers and collects +// the edges buildReferences emits. +func (b *Builder) buildEntityEventHandlers() error { + domainModels, err := b.cachedDomainModels() + if err != nil { + return err + } + + stmt, err := b.tx.Prepare(` + INSERT INTO entity_event_handlers_data + (Id, EntityId, EntityQualifiedName, ModuleName, Moment, Event, + Microflow, RaiseErrorOnFalse, PassEventObject, ProjectId, SnapshotId) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `) + if err != nil { + return err + } + defer stmt.Close() + + projectID, snapshotID := b.snapshotMeta() + + count := 0 + for _, dm := range domainModels { + moduleID := b.hierarchy.findModuleID(dm.ContainerID) + moduleName := b.hierarchy.getModuleName(moduleID) + + for _, entity := range dm.Entities { + for _, row := range entityEventHandlerRows(entity, moduleName) { + if _, err := stmt.Exec( + row.id, row.entityID, row.entityQualifiedName, row.moduleName, + row.moment, row.event, row.microflow, + boolToInt(row.raiseErrorOnFalse), boolToInt(row.passEventObject), + projectID, snapshotID, + ); err != nil { + return err + } + b.eventHandlerRefs = append(b.eventHandlerRefs, eventHandlerRef{ + entityQualifiedName: row.entityQualifiedName, + moduleName: row.moduleName, + microflow: row.microflow, + }) + count++ + } + } + } + + b.report("Entity Event Handlers", count) + return nil +} diff --git a/mdl/catalog/builder_entity_events_test.go b/mdl/catalog/builder_entity_events_test.go new file mode 100644 index 0000000000..65ea791080 --- /dev/null +++ b/mdl/catalog/builder_entity_events_test.go @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// eventFixture is an entity carrying one handler per moment/event combination +// that matters, plus the two cases that must be skipped. +func eventFixture() *domainmodel.Entity { + e := &domainmodel.Entity{ + Name: "Order", + EventHandlers: []*domainmodel.EventHandler{ + {Moment: "Before", Event: "Commit", MicroflowName: "Sales.BCO_Order_Validate", + RaiseErrorOnFalse: true, PassEventObject: true}, + {Moment: "After", Event: "Commit", MicroflowName: "Sales.ACO_Order_Notify", + PassEventObject: true}, + {Moment: "Before", Event: "Delete", MicroflowName: "Sales.BDE_Order_Guard", + RaiseErrorOnFalse: true}, + {Moment: "After", Event: "Create", MicroflowName: "Sales.ACR_Order_Defaults"}, + // A handler naming no microflow is a hole in the document, not a + // reference. A row for it would put an empty TargetName in refs. + {Moment: "Before", Event: "Rollback", MicroflowName: ""}, + nil, + }, + } + e.ID = model.ID("entity-order") + for i, eh := range e.EventHandlers { + if eh != nil { + eh.ID = model.ID(string(rune('a'+i)) + "-handler") + } + } + return e +} + +// The flag said "some exist" and nothing else — the same shape +// NavigationProfile.OfflineEntityCount had before CATALOG.OFFLINE_ENTITY_CONFIGS. +// Which moment, which event and which microflow is the whole question, and refs +// has no column for the first two. +func TestEntityEventHandlerRowsCarryMomentAndEvent(t *testing.T) { + rows := entityEventHandlerRows(eventFixture(), "Sales") + + if len(rows) != 4 { + t.Fatalf("got %d rows, want 4 — the microflow-less handler and the nil must be skipped", len(rows)) + } + + want := map[string][2]string{ + "Sales.BCO_Order_Validate": {"Before", "Commit"}, + "Sales.ACO_Order_Notify": {"After", "Commit"}, + "Sales.BDE_Order_Guard": {"Before", "Delete"}, + "Sales.ACR_Order_Defaults": {"After", "Create"}, + } + for _, r := range rows { + w, ok := want[r.microflow] + if !ok { + t.Errorf("unexpected row for %q", r.microflow) + continue + } + if r.moment != w[0] || r.event != w[1] { + t.Errorf("%s = %s %s, want %s %s", r.microflow, r.moment, r.event, w[0], w[1]) + } + if r.entityQualifiedName != "Sales.Order" { + t.Errorf("%s entity = %q, want Sales.Order", r.microflow, r.entityQualifiedName) + } + if r.moduleName != "Sales" { + t.Errorf("%s module = %q, want Sales", r.microflow, r.moduleName) + } + delete(want, r.microflow) + } + for missing := range want { + t.Errorf("no row for %s", missing) + } + + // RaiseErrorOnFalse is the difference between a handler that can veto a + // commit and one that only observes it. Dropping it would make the table + // agree with itself and disagree with the model. + for _, r := range rows { + switch r.microflow { + case "Sales.BCO_Order_Validate": + if !r.raiseErrorOnFalse || !r.passEventObject { + t.Errorf("BCO handler = raise:%v pass:%v, want both true", r.raiseErrorOnFalse, r.passEventObject) + } + case "Sales.ACO_Order_Notify": + if r.raiseErrorOnFalse { + t.Error("ACO handler must not raise on false — it runs after the commit") + } + } + } + + // Mendix spells the rollback event with a capital B, which + // generated/metamodel confirms and which disagrees with every neighbouring + // enum in that file. The value is stored verbatim: normalising it here + // would make the table agree with itself and disagree with the model. + rb := &domainmodel.Entity{Name: "Order", EventHandlers: []*domainmodel.EventHandler{ + {Moment: "Before", Event: domainmodel.EventTypeRollback, MicroflowName: "Sales.BRB_Order"}, + }} + if got := entityEventHandlerRows(rb, "Sales"); len(got) != 1 || got[0].event != "RollBack" { + t.Errorf("rollback event stored as %q, want %q verbatim", got[0].event, "RollBack") + } + + if rows := entityEventHandlerRows(nil, "Sales"); rows != nil { + t.Errorf("nil entity = %v, want nil", rows) + } +} + +// The defect this fixes, stated as the query three tools ask. The control is +// the same catalog WITHOUT the event edges: it must report the handler +// microflow as dead, or the assertion below proves nothing. +func TestEventEdgeClearsTheDeadAssetVerdict(t *testing.T) { + seed := func(t *testing.T, withEdges bool) []string { + t.Helper() + cat, err := New() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { cat.Close() }) + db := cat.CatalogDB() + + if _, err := db.Exec(`INSERT INTO snapshots (SnapshotId, ProjectId) VALUES ('s','p')`); err != nil { + t.Fatal(err) + } + if _, err := db.Exec( + `INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, EntityType, + HasEventHandlers, ProjectId, SnapshotId) + VALUES ('entity-order', 'Order', 'Sales.Order', 'Sales', 'PERSISTENT', 1, 'p', 's')`); err != nil { + t.Fatal(err) + } + + rows := entityEventHandlerRows(eventFixture(), "Sales") + for _, r := range rows { + name := r.microflow[len("Sales."):] + if _, err := db.Exec( + `INSERT INTO microflows_data (Id, Name, QualifiedName, ModuleName, + MicroflowType, ProjectId, SnapshotId) + VALUES (?, ?, ?, 'Sales', 'MICROFLOW', 'p', 's')`, + r.microflow, name, r.microflow); err != nil { + t.Fatal(err) + } + if !withEdges { + continue + } + if _, err := db.Exec( + `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, + TargetName, RefKind, ModuleName, ProjectId, SnapshotId) + VALUES ('ENTITY', '', ?, 'MICROFLOW', '', ?, ?, 'Sales', 'p', 's')`, + r.entityQualifiedName, r.microflow, RefKindEvent); err != nil { + t.Fatal(err) + } + } + + res, err := cat.Query( + `SELECT QualifiedName FROM graph_dead_assets WHERE ObjectType = 'MICROFLOW' ORDER BY QualifiedName`) + if err != nil { + t.Fatal(err) + } + var dead []string + for _, row := range res.Rows { + dead = append(dead, row[0].(string)) + } + return dead + } + + // Control: without the edge, every handler microflow is reported dead — + // the bug as filed (mendixlabs/mxcli#1127). + if dead := seed(t, false); len(dead) != 4 { + t.Fatalf("control: %d dead microflows, want 4 — if the control does not "+ + "reproduce the defect, the assertion below detects nothing", len(dead)) + } + + if dead := seed(t, true); len(dead) != 0 { + t.Errorf("with the event edge, %v are still reported dead", dead) + } +} + +// Reusing an existing kind would make `show references` say the wrong thing +// about how the entity uses the microflow. +func TestRefKindEventIsItsOwnKind(t *testing.T) { + for _, other := range []string{ + RefKindCall, RefKindCalculate, RefKindSchedule, RefKindChange, + RefKindDelete, RefKindAction, RefKindSettings, + } { + if RefKindEvent == other { + t.Errorf("RefKindEvent collides with %q", other) + } + } + if RefKindEvent != "event" { + t.Errorf("RefKindEvent = %q; the value appears in user-facing output and in "+ + "the bundled Starlark rules", RefKindEvent) + } +} + +// The three consumers of the reference graph keep independent lists, and none +// shares the other's. A kind added to the builder alone is an edge nothing reads. +func TestEventKindReachesTheAssetGraph(t *testing.T) { + if !contains(graphRefKinds, RefKindEvent) { + t.Error("graphRefKinds is missing 'event' — the handler microflow stays " + + "outside the analysis graph even though the edge exists") + } +} diff --git a/mdl/catalog/builder_graph.go b/mdl/catalog/builder_graph.go index f02f77da4b..699a6cc601 100644 --- a/mdl/catalog/builder_graph.go +++ b/mdl/catalog/builder_graph.go @@ -18,10 +18,20 @@ import ( var graphRefKinds = []string{ "call", "retrieve", "create", "change", "delete", "associate", "generalize", "parameter", "return", - // A scheduled event is an entry point: the microflow it runs is reachable - // even though nothing calls it. Without this kind, GRAPH_DEAD_ASSETS reports - // every scheduled microflow as dead. + // Entry points: something outside the call graph invokes these, so the + // microflow they run is reachable even though nothing in the model calls it. Leaving one out does + // not hide it from GRAPH_DEAD_ASSETS — that view asks only whether ANY refs + // row targets the name, whatever its kind — but it does cut the microflow out + // of the analysis graph, so communities, layers, cycles and centrality all + // see the API and scheduling surface as unreachable roots. + // schedule a scheduled event runs it + // publish a published REST operation runs it + // event an entity event handler runs it on every commit/delete — the + // entity rather than the platform, but reached the same way, and + // structural rather than UI coupling "schedule", + "publish", + "event", } // graphRefKindsSQL renders graphRefKinds as a quoted SQL IN list, so the schema diff --git a/mdl/catalog/builder_references.go b/mdl/catalog/builder_references.go index f706777107..8f47dacb1c 100644 --- a/mdl/catalog/builder_references.go +++ b/mdl/catalog/builder_references.go @@ -37,6 +37,8 @@ const ( RefKindWidget = "widget" // Page/snippet uses a pluggable or custom widget RefKindSettings = "settings" // A project setting names a microflow RefKindSync = "sync" // An offline navigation profile synchronizes an entity + RefKindPublish = "publish" // A published REST operation runs a microflow + RefKindEvent = "event" // An entity event handler runs a microflow ) // collectActionActivities returns all ActionActivity objects from an ObjectCollection, @@ -568,6 +570,12 @@ func (b *Builder) buildReferences() error { // pattern. refCount += b.extractRegexRuleRefs(stmt, projectID, snapshotID) + // Entity event handlers run a microflow on every create/commit/delete of + // the entity. Same class as the scheduled-event edge above, and worse in + // degree: a handler runs on every commit, so the false "dead" verdict lands + // on code that is hotter than most of what IS reported as live. + refCount += b.extractEventHandlerRefs(stmt, projectID, snapshotID) + // Three project settings name a microflow the runtime calls. Same class as // the scheduled-event edge above and found the same way: a microflow wired as // AfterStartupMicroflow reported no callers and no references, QUAL004 said @@ -576,6 +584,13 @@ func (b *Builder) buildReferences() error { // the runtime refused to start (ako/CapTrackV4 049). refCount += b.extractProjectSettingsRefs(stmt, projectID, snapshotID) + // A published REST operation runs a microflow the same way a scheduled event + // does — the platform invokes it, so nothing in the model calls it. Without + // this edge every microflow behind the public API reads as dead: on the + // reported model, 92 of 93 operations name a microflow and all 92 were listed + // by GRAPH_DEAD_ASSETS, whose advice is to delete them (#1126). + refCount += b.extractPublishedRestRefs(stmt, projectID, snapshotID) + b.report("References", refCount) return nil } @@ -615,6 +630,57 @@ func (b *Builder) extractScheduledEventRefs(stmt *sql.Stmt, projectID, snapshotI return count } +// extractPublishedRestRefs emits one `publish` edge per published REST operation +// that names a microflow, from the operation to the microflow it runs. +// +// The source is the OPERATION, not the service, because that is the granularity +// the question is asked at: `show references to ` should name the one +// endpoint that reaches it, not the service holding thirty of them. The +// operation's catalog id travels with the edge so the path and summary are one +// join away. +// +// The edges are collected by buildPublishedRestServices, which runs earlier in +// the same transaction. +func (b *Builder) extractPublishedRestRefs(stmt *sql.Stmt, projectID, snapshotID string) int { + count := 0 + for _, r := range b.publishedRestRefs { + if _, err := stmt.Exec( + "PUBLISHED_REST_OPERATION", r.sourceID, r.qualifiedName, + "MICROFLOW", "", r.microflow, + RefKindPublish, r.moduleName, projectID, snapshotID, + ); err == nil { + count++ + } + } + return count +} + +// extractEventHandlerRefs emits one `event` edge per entity event handler, from +// the entity to the microflow it runs. +// +// The edge carries neither the moment nor the event — refs has no column for +// them, and inventing a kind per combination ("before_commit", "after_delete") +// would put eight kinds into every consumer's list to say one thing. Which +// moment and which event is CATALOG.ENTITY_EVENT_HANDLERS' question; this edge +// answers "is the microflow reachable", which is the one three tools were +// getting wrong. +// +// The edges are collected by buildEntityEventHandlers, which runs earlier in +// the same transaction. +func (b *Builder) extractEventHandlerRefs(stmt *sql.Stmt, projectID, snapshotID string) int { + count := 0 + for _, r := range b.eventHandlerRefs { + if _, err := stmt.Exec( + "ENTITY", "", r.entityQualifiedName, + "MICROFLOW", "", r.microflow, + RefKindEvent, r.moduleName, projectID, snapshotID, + ); err == nil { + count++ + } + } + return count +} + // extractMenuItemRefs extracts page and microflow references from menu items recursively. func (b *Builder) extractMenuItemRefs(stmt *sql.Stmt, items []*types.NavMenuItem, sourceName, projectID, snapshotID string) int { refCount := 0 diff --git a/mdl/catalog/builder_rest.go b/mdl/catalog/builder_rest.go index e230e7b664..767a27601a 100644 --- a/mdl/catalog/builder_rest.go +++ b/mdl/catalog/builder_rest.go @@ -5,6 +5,7 @@ package catalog import ( "crypto/sha256" "fmt" + "strings" ) // buildRestClients populates the rest_clients and rest_operations catalog tables. @@ -104,6 +105,25 @@ func (b *Builder) buildRestClients() error { return nil } +// publishedRestRef is one published REST operation → microflow edge. sourceID is +// the operation's synthetic catalog id, so the emitted edge joins back to +// published_rest_operations_data rather than only naming the operation in prose. +type publishedRestRef struct{ sourceID, qualifiedName, moduleName, microflow string } + +// publishedRestOpName renders the name a published operation is known by in the +// reference graph. The empty parts are dropped rather than left as runs of +// spaces: an operation on a resource's own root has no path, and a name ending +// in whitespace is one a user cannot retype. +func publishedRestOpName(service, resource, method, path string) string { + parts := make([]string, 0, 4) + for _, p := range []string{service, resource, method, path} { + if p != "" { + parts = append(parts, p) + } + } + return strings.Join(parts, " ") +} + // buildPublishedRestServices populates the published_rest_services and published_rest_operations catalog tables. func (b *Builder) buildPublishedRestServices() error { services, err := b.reader.ListPublishedRestServices() @@ -192,6 +212,21 @@ func (b *Builder) buildPublishedRestServices() error { return err } totalOps++ + + // An operation with no microflow is a real shape — Mendix allows + // one while the service is being built — and an edge to the empty + // name would collide with every other unnamed target in refs. + if op.Microflow != "" { + b.publishedRestRefs = append(b.publishedRestRefs, publishedRestRef{ + sourceID: opID, + // The resource is part of what makes an operation unique: + // two resources of one service can both expose GET on the + // same relative path. + qualifiedName: publishedRestOpName(qualifiedName, res.Name, op.HTTPMethod, op.Path), + moduleName: moduleName, + microflow: op.Microflow, + }) + } } } } diff --git a/mdl/catalog/builder_rest_refs_test.go b/mdl/catalog/builder_rest_refs_test.go new file mode 100644 index 0000000000..858251c569 --- /dev/null +++ b/mdl/catalog/builder_rest_refs_test.go @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" +) + +// A published REST operation is an entry point of the same shape as a scheduled +// event: a document that NAMES a microflow and runs it, with no call activity +// anywhere in the model. The catalog recorded the binding in +// published_rest_operations_data.Microflow and emitted no `refs` edge, so every +// microflow whose only caller is an operation had zero inbound references — +// listed by GRAPH_DEAD_ASSETS, "(no callers found)" from SHOW CALLERS, and +// QUAL004 "not called from anywhere. Remove if unused." on the back end of the +// public API. Measured on the reporter's model: 92 of 93 operations name a +// microflow and all 92 were reported dead (mendixlabs/mxcli#1126). + +const restRefsModuleID = model.ID("mod-sales") + +// restFixture runs the two real halves of the path — buildPublishedRestServices, +// which collects the edges, and extractPublishedRestRefs, which emits them — over +// one service, and returns the catalog they wrote into. +func restFixture(t *testing.T, svc *model.PublishedRestService) *Catalog { + t.Helper() + + cat, err := New() + if err != nil { + t.Fatalf("new catalog: %v", err) + } + t.Cleanup(func() { cat.Close() }) + + tx, err := cat.db.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + + b := &Builder{ + catalog: cat, + reader: &mock.MockBackend{ + ListPublishedRestServicesFunc: func() ([]*model.PublishedRestService, error) { + return []*model.PublishedRestService{svc}, nil + }, + }, + snapshot: &Snapshot{ID: "snap-1"}, + hierarchy: &hierarchy{ + moduleIDs: map[model.ID]bool{restRefsModuleID: true}, + moduleNames: map[model.ID]string{restRefsModuleID: "Sales"}, + containerParent: map[model.ID]model.ID{}, + folderNames: map[model.ID]string{}, + }, + tx: tx, + } + + if err := b.buildPublishedRestServices(); err != nil { + t.Fatalf("buildPublishedRestServices: %v", err) + } + + stmt, err := tx.Prepare(` + INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ModuleName, ProjectId, SnapshotId) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `) + if err != nil { + t.Fatalf("prepare refs: %v", err) + } + projectID, snapshotID := b.snapshotMeta() + b.extractPublishedRestRefs(stmt, projectID, snapshotID) + stmt.Close() + + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + return cat +} + +// oneService returns a service with a single resource holding the given +// operations, containered directly in the module. +func oneService(ops ...*model.PublishedRestOperation) *model.PublishedRestService { + svc := &model.PublishedRestService{ + ContainerID: restRefsModuleID, + Name: "OrdersApi", + Path: "orders/v1", + Resources: []*model.PublishedRestResource{ + {Name: "Order", Operations: ops}, + }, + } + svc.ID = model.ID("svc-1") + return svc +} + +func queryRows(t *testing.T, cat *Catalog, q string) [][]any { + t.Helper() + res, err := cat.Query(q) + if err != nil { + t.Fatalf("query %q: %v", q, err) + } + return res.Rows +} + +// TestPublishedRestOperationEmitsPublishEdge pins the edge itself: its endpoints, +// its kind, and that it carries the operation's own id so it joins back to +// published_rest_operations_data. +func TestPublishedRestOperationEmitsPublishEdge(t *testing.T) { + cat := restFixture(t, oneService(&model.PublishedRestOperation{ + HTTPMethod: "GET", + Path: "{id}", + Microflow: "Sales.GetOrder", + })) + + rows := queryRows(t, cat, `SELECT SourceType, SourceName, TargetType, TargetName, RefKind, ModuleName FROM refs`) + if len(rows) != 1 { + t.Fatalf("want exactly 1 refs row, got %d: %v", len(rows), rows) + } + got := rows[0] + want := []any{"PUBLISHED_REST_OPERATION", "Sales.OrdersApi Order GET {id}", "MICROFLOW", "Sales.GetOrder", "publish", "Sales"} + for i := range want { + if got[i] != want[i] { + t.Errorf("column %d = %v, want %v", i, got[i], want[i]) + } + } + + // The edge must be joinable to the operation it came from, so a consumer can + // get from "who calls this microflow" to the operation's path and summary. + joined := queryRows(t, cat, ` + SELECT COUNT(*) FROM refs r + JOIN published_rest_operations_data o ON o.Id = r.SourceId + WHERE r.RefKind = 'publish'`) + if joined[0][0].(int64) != 1 { + t.Errorf("the publish edge does not join to published_rest_operations_data on SourceId; "+ + "got %v matches", joined[0][0]) + } +} + +// An operation with no microflow is a real shape — Mendix allows a +// published operation bound to nothing while it is being built — and must not +// produce an edge to the empty name, which would collide with every other +// unnamed target in the table. +func TestPublishedRestOperationWithoutMicroflowEmitsNothing(t *testing.T) { + cat := restFixture(t, oneService( + &model.PublishedRestOperation{HTTPMethod: "GET", Path: "{id}", Microflow: "Sales.GetOrder"}, + &model.PublishedRestOperation{HTTPMethod: "POST", Path: ""}, + )) + + rows := queryRows(t, cat, `SELECT COUNT(*) FROM published_rest_operations_data`) + if rows[0][0].(int64) != 2 { + t.Fatalf("control: want both operations catalogued, got %v", rows[0][0]) + } + rows = queryRows(t, cat, `SELECT COUNT(*) FROM refs`) + if rows[0][0].(int64) != 1 { + t.Errorf("want 1 edge for the 2 operations (only one names a microflow), got %v", rows[0][0]) + } +} + +// TestPublishedRestMicroflowIsNotDead is the reported symptom. GRAPH_DEAD_ASSETS +// is kind-agnostic — it asks only whether ANY refs row targets the name — so the +// edge alone clears it, whatever kind it carries. +// +// The unreferenced microflow in the same fixture is the control: without it a +// fix that emptied the view entirely would pass. +func TestPublishedRestMicroflowIsNotDead(t *testing.T) { + cat := restFixture(t, oneService(&model.PublishedRestOperation{ + HTTPMethod: "GET", + Path: "{id}", + Microflow: "Sales.GetOrder", + })) + + for _, mf := range []string{"GetOrder", "Orphan"} { + if _, err := cat.db.Exec( + `INSERT INTO microflows_data (Id, Name, QualifiedName, ModuleName, MicroflowType) + VALUES (?, ?, ?, 'Sales', 'MICROFLOW')`, + "mf-"+mf, mf, "Sales."+mf); err != nil { + t.Fatalf("insert microflow: %v", err) + } + } + + dead := map[string]bool{} + for _, row := range queryRows(t, cat, `SELECT QualifiedName FROM graph_dead_assets`) { + dead[row[0].(string)] = true + } + if !dead["Sales.Orphan"] { + t.Fatal("control failed: a microflow nothing references is not reported dead, " + + "so this test cannot detect the bug") + } + if dead["Sales.GetOrder"] { + t.Error("a microflow invoked by a published REST operation is reported dead — " + + "the dead list recommends deleting the back end of the public API (#1126)") + } +} diff --git a/mdl/catalog/catalog.go b/mdl/catalog/catalog.go index 6e8228fb9c..516de09513 100644 --- a/mdl/catalog/catalog.go +++ b/mdl/catalog/catalog.go @@ -132,6 +132,7 @@ func (c *Catalog) Tables() []string { "CATALOG.CONSUMED_MCP_SERVICES", "CATALOG.NAVIGATION_PROFILES", "CATALOG.OFFLINE_ENTITY_CONFIGS", + "CATALOG.ENTITY_EVENT_HANDLERS", "CATALOG.ACTIVITIES", "CATALOG.WIDGETS", "CATALOG.WIDGET_DEFINITIONS", diff --git a/mdl/catalog/lint_rule_vocabulary_test.go b/mdl/catalog/lint_rule_vocabulary_test.go index 30c0083a81..7b7e6708f1 100644 --- a/mdl/catalog/lint_rule_vocabulary_test.go +++ b/mdl/catalog/lint_rule_vocabulary_test.go @@ -98,6 +98,7 @@ func TestQUAL004EntryKindsAreRealRefKinds(t *testing.T) { RefKindParameter, RefKindAction, RefKindHomePage, RefKindLoginPage, RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, + RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, } { known[k] = true } @@ -123,7 +124,11 @@ func TestQUAL004CountsEveryEntryPointKind(t *testing.T) { for _, want := range []string{ RefKindCall, RefKindSchedule, RefKindDatasource, RefKindAction, RefKindCalculate, - RefKindSettings, + RefKindSettings, RefKindPublish, + // An entity event handler runs on every commit/delete of its entity. + // Of every kind in this list it is the one whose absence flags the + // hottest code (mendixlabs/mxcli#1127). + RefKindEvent, } { if !contains(starListItems(src, "MICROFLOW_ENTRY_KINDS"), want) { t.Errorf("MICROFLOW_ENTRY_KINDS is missing %q — a microflow reached only that way "+ diff --git a/mdl/catalog/tables.go b/mdl/catalog/tables.go index 69d1225fa6..84aca3aee0 100644 --- a/mdl/catalog/tables.go +++ b/mdl/catalog/tables.go @@ -7,6 +7,12 @@ package catalog // // History: // +// 13 — entity_event_handlers_data + view, and the `event` edge in refs +// (ENTITY -> MICROFLOW). Same reason as 11: refs are only written by +// REFRESH CATALOG FULL, so without the bump a cached catalog keeps +// reporting every handler-only microflow in GRAPH_DEAD_ASSETS and +// answering `show callers` with "(no callers found)" — the wrong answer +// this change exists to stop, served from a stale cache. // 12 — HasCreatedDate / HasChangedDate / HasOwner / HasChangedBy on entities. // A CREATE TABLE IF NOT EXISTS does not add a column to a cached // catalog, so without the bump every query naming one fails with "no @@ -735,6 +741,34 @@ func (c *Catalog) createTables() error { )`, viewWithFullSnapshot("offline_entity_configs"), + // entity_event_handlers — one row per entity event handler. + // CATALOG.ENTITIES.HasEventHandlers is a flag: it says some exist and + // nothing else. Which moment, which event and which microflow is the + // whole question, and refs has no column for the first two — a `before + // commit` handler that returns false blocks the commit, an `after + // delete` one cannot. The edge says the microflow is reachable; this + // table says what it does. + // + // Event holds the value Mendix stores: Create/Commit/Delete/RollBack. + // The capital B in RollBack is Mendix's, confirmed against + // generated/metamodel, and disagrees with every neighbouring enum + // there — a query spelling it `Rollback` returns zero rows, not an + // error. + `CREATE TABLE IF NOT EXISTS entity_event_handlers_data ( + Id TEXT, + EntityId TEXT, + EntityQualifiedName TEXT, + ModuleName TEXT, + Moment TEXT, + Event TEXT, + Microflow TEXT, + RaiseErrorOnFalse INTEGER DEFAULT 0, + PassEventObject INTEGER DEFAULT 0, + ProjectId TEXT, + SnapshotId TEXT + )`, + viewWithFullSnapshot("entity_event_handlers"), + // Already-clean tables (no denormalized columns) — kept as plain tables. `CREATE TABLE IF NOT EXISTS navigation_menu_items ( Id INTEGER PRIMARY KEY AUTOINCREMENT, diff --git a/mdl/executor/cmd_microflows_build.go b/mdl/executor/cmd_microflows_build.go index db5976396a..21b4a5a82f 100644 --- a/mdl/executor/cmd_microflows_build.go +++ b/mdl/executor/cmd_microflows_build.go @@ -124,6 +124,29 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b // SECURITY setting, so an absent annotation must preserve a stored true // rather than widening what the microflow may read and write. existingApplyEntityAccess := false + // The deep link (Mendix 10.6+). MDL has no syntax for it, so a rewrite + // carries the stored value rather than rebuilding it — hardcoding "" is + // what deleted it on every CREATE OR MODIFY (#1120). + var existingURL string + var existingURLSearchParams []string + // Studio Pro's "Export level". Same rule: no MDL syntax, so a rewrite + // carries it. Empty means "no stored microflow", which the writer turns + // into the "Hidden" default. + var existingExportLevel string + // Concurrency. None of these four has MDL syntax either, and the rebuild + // used to write its own values over all of them. The initial values here + // are the defaults for a NEW microflow, which is why no separate "preserve" + // flag is needed: a stored microflow overwrites them below, and anything + // else is a create. + // + // The direction is what made this one invisible. The rebuild hardcoded + // `true`, so a microflow that DISALLOWED concurrent execution came back + // allowing it — and CE4899 only fires on disallow-without-a-message, so + // removing the app's concurrency protection reported nothing at all. + existingAllowConcurrentExecution := true + existingMarkAsUsed := false + var existingConcurrencyErrorMessage *model.Text + var existingConcurrencyErrorMicroflow string var existingDocumentation string preserveDocumentation := false var existingActionInfo, existingWorkflowInfo *types.MicroflowActionInfo @@ -149,6 +172,13 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b preserveAllowedRoles = true existingExcluded = existing.Excluded existingApplyEntityAccess = existing.ApplyEntityAccess + existingExportLevel = existing.ExportLevel + existingAllowConcurrentExecution = existing.AllowConcurrentExecution + existingMarkAsUsed = existing.MarkAsUsed + existingConcurrencyErrorMessage = existing.ConcurrencyErrorMessage + existingConcurrencyErrorMicroflow = existing.ConcurrencyErrorMicroflow + existingURL = existing.URL + existingURLSearchParams = append([]string(nil), existing.URLSearchParameters...) // The toolbox entries hold four PNG bitmaps MDL cannot name, so a // rewrite carries them rather than rebuilding from the clause. existingActionInfo = existing.MicroflowActionInfo @@ -203,13 +233,18 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b BaseElement: model.BaseElement{ ID: microflowID, }, - ContainerID: containerID, - Name: s.Name.Name, - Documentation: s.Documentation, - AllowConcurrentExecution: true, // Default: allow concurrent execution - MarkAsUsed: false, - Excluded: s.Excluded || existingExcluded, - ApplyEntityAccess: carriedApplyEntityAccess(s.ApplyEntityAccess, existingApplyEntityAccess), + ContainerID: containerID, + Name: s.Name.Name, + Documentation: s.Documentation, + AllowConcurrentExecution: existingAllowConcurrentExecution, // new microflows default to true + MarkAsUsed: existingMarkAsUsed, + Excluded: s.Excluded || existingExcluded, + ApplyEntityAccess: carriedApplyEntityAccess(s.ApplyEntityAccess, existingApplyEntityAccess), + ConcurrencyErrorMessage: existingConcurrencyErrorMessage, + ConcurrencyErrorMicroflow: existingConcurrencyErrorMicroflow, + ExportLevel: existingExportLevel, + URL: existingURL, + URLSearchParameters: existingURLSearchParams, } if preserveDocumentation { mf.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existingDocumentation) diff --git a/mdl/executor/cmd_microflows_show.go b/mdl/executor/cmd_microflows_show.go index 14a1aedaf7..7a63cbd35a 100644 --- a/mdl/executor/cmd_microflows_show.go +++ b/mdl/executor/cmd_microflows_show.go @@ -622,6 +622,24 @@ func renderMicroflowMDL( if mf.ApplyEntityAccess && flowType == "microflow" { lines = append(lines, "@applyentityaccess") } + // Studio Pro's "Export level". Only worth a line when it is NOT the default + // — every document in every module measured stores "Hidden", so emitting it + // unconditionally would add a comment to every describe to say nothing. + if mf.ExportLevel != "" && mf.ExportLevel != "Hidden" && flowType == "microflow" { + lines = append(lines, fmt.Sprintf( + "-- Export level: %s (MDL cannot author one. Kept when this microflow "+ + "is rewritten, NOT copied to a new one — set it in Studio Pro.)", mf.ExportLevel)) + } + // The deep link (Mendix 10.6+) has no MDL spelling at all, so it cannot be + // emitted as re-executable text. A rewrite preserves it (#1120), but a + // describe -> rename -> exec COPY has nothing to preserve from — same gap + // the annotation above notes, one step further along. Say so rather than + // producing output that silently omits it. + if mf.URL != "" && flowType == "microflow" { + lines = append(lines, fmt.Sprintf( + "-- URL: %s (deep link; MDL cannot author one. Kept when this "+ + "microflow is rewritten, NOT copied to a new one — set it in Studio Pro.)", mf.URL)) + } qualifiedName := name.Module + "." + name.Name if len(mf.Parameters) > 0 { diff --git a/mdl/executor/cmd_pages_create_v3.go b/mdl/executor/cmd_pages_create_v3.go index d27a5f0c62..154adc2f30 100644 --- a/mdl/executor/cmd_pages_create_v3.go +++ b/mdl/executor/cmd_pages_create_v3.go @@ -21,7 +21,12 @@ func execCreatePageV3(ctx *ExecContext, s *ast.CreatePageStmtV3) error { return mdlerrors.NewNotConnectedWrite() } - // Version pre-check: page parameters require 11.0+ + // Version pre-check: page parameters exist from Mendix 9.4, not 11.0. The + // registry used to say 11.0 — a figure that came from an illustrative sample + // table in PROPOSAL_version_aware_agent_support.md, never from a measurement — + // which locked every 10.x project out of parameterised pages entirely + // (mendixlabs/mxcli#1121). The gate stays because 9.0–9.3 genuinely has no + // Pages$PageParameter; only the floor and the hint were wrong. if len(s.Parameters) > 0 { if err := checkFeature(ctx, "pages", "page_parameters", "create page with parameters", diff --git a/mdl/executor/cmd_search.go b/mdl/executor/cmd_search.go index de3e283dda..557fd62777 100644 --- a/mdl/executor/cmd_search.go +++ b/mdl/executor/cmd_search.go @@ -20,15 +20,24 @@ import ( // // A false negative here reads as "nothing uses this", which is the answer // somebody acts on before deleting a document — so the set errs toward -// including a kind rather than omitting it. 'schedule' is the same shape: a -// microflow run only by a scheduled event reported "(no callers found)", was -// listed in GRAPH_DEAD_ASSETS, and drew QUAL004 "is not called from anywhere" -// with the suggestion "Remove if unused" — on a microflow that runs nightly. +// including a kind rather than omitting it. +// +// The recurring shape is an ENTRY POINT: something outside the call graph +// invokes it, so nothing in the model calls it. 'schedule' was the first (a +// microflow run only by a scheduled event reported "(no callers found)" on a job +// that runs nightly); 'publish' is a published REST operation (#1126); 'event' +// is an entity event handler, run by the entity on commit or delete; 'settings' +// is a microflow wired as after-startup, before-shutdown or health check, whose +// edge shipped in v0.22.0 and was added to QUAL004 but not here, so +// `show callers` stayed blind to it. Every new way for something to run a +// microflow belongs in this list, in graphRefKinds, and in QUAL004's +// MICROFLOW_ENTRY_KINDS — three consumers, none of which shares the others' +// list. // // Deliberately excluded: 'datasource', 'parameter', 'return', 'retrieve', -// 'create', 'change', 'delete', 'associate', 'generalize' and 'layout'. Those -// are uses of a TYPE or a LAYOUT, not invocations, and folding them in would -// make `show callers of ` a synonym for `show references to`. +// 'create', 'change', 'delete', 'associate', 'generalize', 'layout' and 'sync'. +// Those are uses of a TYPE or a LAYOUT, not invocations, and folding them in +// would make `show callers of ` a synonym for `show references to`. var callerRefKinds = []string{ RefKindCallerCall, // microflow/nanoflow call activity RefKindCallerAction, // widget action: button, on-change, on-click @@ -38,6 +47,9 @@ var callerRefKinds = []string{ RefKindCallerLoginPage, RefKindCallerMenuItem, RefKindCallerSchedule, // scheduled event: the microflow it runs + RefKindCallerPublish, // published REST operation: the microflow behind the endpoint + RefKindCallerSettings, // project setting: after-startup, before-shutdown, health check + RefKindCallerEvent, // entity event handler: the microflow it runs } // Kind literals, kept next to the set that uses them so the SQL below cannot @@ -51,6 +63,9 @@ const ( RefKindCallerLoginPage = "login_page" RefKindCallerMenuItem = "menu_item" RefKindCallerSchedule = "schedule" + RefKindCallerPublish = "publish" + RefKindCallerSettings = "settings" + RefKindCallerEvent = "event" ) // callerRefKindsSQL renders callerRefKinds as a SQL IN list. diff --git a/mdl/executor/cmd_search_callers_test.go b/mdl/executor/cmd_search_callers_test.go index a9998eb775..877d2facbf 100644 --- a/mdl/executor/cmd_search_callers_test.go +++ b/mdl/executor/cmd_search_callers_test.go @@ -31,6 +31,13 @@ func TestCallerRefKinds(t *testing.T) { "home_page", // navigation "login_page", // "menu_item", // + "schedule", // entry point: a scheduled event runs the microflow + "publish", // entry point: a published REST operation runs the microflow (#1126) + "settings", // entry point: after-startup / before-shutdown / health check + // An entity event handler runs its microflow on every create/commit/ + // delete of the entity. Omitting it reported the hottest code in the + // app as uncalled (mendixlabs/mxcli#1127). + "event", } { if !in[k] { t.Errorf("%q means one document invokes another and must count as a caller — "+ @@ -44,6 +51,9 @@ func TestCallerRefKinds(t *testing.T) { for _, k := range []string{ "datasource", "parameter", "return", "retrieve", "create", "change", "delete", "associate", "generalize", "layout", + // `sync` reads as an entry point and is not one: an offline profile + // names an ENTITY it downloads, which is a use of a type. + "sync", } { if in[k] { t.Errorf("%q is a use of a type or layout, not an invocation — including it "+ diff --git a/mdl/executor/microflow_carried_properties_test.go b/mdl/executor/microflow_carried_properties_test.go new file mode 100644 index 0000000000..d543202406 --- /dev/null +++ b/mdl/executor/microflow_carried_properties_test.go @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// Executor-level coverage for the microflow properties MDL cannot author and a +// rewrite therefore has to carry: the deep link, the export level, and the four +// concurrency settings. All were hardcoded in buildMicroflowFromStmt's rebuild +// struct or in microflowToGen, and all were lost with every checker green. +// +// Each has a paired control here — a CREATE must not acquire what only a stored +// microflow can supply — because "preserve the stored value" and "invent one" +// are the same edit seen from opposite sides. + +// TestCreateOrModifyMicroflow_PreservesDeepLinkURL is the executor half of +// #1120: a statement that never mentions the deep link must not clear one. +// +// A microflow's URL (Mendix 10.6+) has no MDL spelling, so `CREATE OR MODIFY +// MICROFLOW` rebuilt it from the AST default — an empty string — and the deep +// link was gone. The class is the one ADR-0005 calls guard-don't-drop, but with +// nothing to guard against: unlike a queued call or an unwritable REST body, +// a microflow with no URL is a perfectly valid document, so `mxcli check`, +// `mx check` and mxbuild all report success before and after. Only Studio Pro +// shows the loss. Preserving is therefore the whole remedy; there is no +// refusal to fall back on. +func TestCreateOrModifyMicroflow_PreservesDeepLinkURL(t *testing.T) { + const moduleID = model.ID("module-1") + stored := []*microflows.Microflow{{ + BaseElement: model.BaseElement{ID: "mf-item"}, + ContainerID: moduleID, + Name: "ACT_Item", + URL: "item/{Key}", + URLSearchParameters: []string{"MyModule.ACT_Item.Filter"}, + }} + ctx, written := microflowWriteProbe(t, stored, moduleID) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "ACT_Item"}, + CreateOrModify: true, + } + if err := execCreateMicroflow(ctx, stmt); err != nil { + t.Fatalf("CREATE OR MODIFY MICROFLOW failed: %v", err) + } + if *written == nil { + t.Fatal("no microflow was written") + } + if got := (*written).URL; got != "item/{Key}" { + t.Errorf("rewrite dropped the deep-link URL: got %q, want %q", got, "item/{Key}") + } + if got := (*written).URLSearchParameters; len(got) != 1 || got[0] != "MyModule.ACT_Item.Filter" { + t.Errorf("rewrite dropped UrlSearchParameters: got %v, want [MyModule.ACT_Item.Filter]", got) + } +} + +// TestCreateMicroflow_InventsNoDeepLinkURL is the control for the test above: a +// microflow that never had a URL must not acquire one, or "preserve the stored +// value" would just be a different silent change in the same place. +func TestCreateMicroflow_InventsNoDeepLinkURL(t *testing.T) { + const moduleID = model.ID("module-1") + ctx, written := microflowWriteProbe(t, nil, moduleID) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "ACT_Fresh"}, + } + if err := execCreateMicroflow(ctx, stmt); err != nil { + t.Fatalf("CREATE MICROFLOW failed: %v", err) + } + if *written == nil { + t.Fatal("no microflow was written") + } + if got := (*written).URL; got != "" { + t.Errorf("a fresh microflow acquired a deep-link URL: %q", got) + } + if got := (*written).URLSearchParameters; len(got) != 0 { + t.Errorf("a fresh microflow acquired UrlSearchParameters: %v", got) + } +} + +// TestCreateOrModifyMicroflow_PreservesExportLevel is the executor half of the +// export-level carry. Same shape as the deep link above and found the same way: +// a constant in the writer that nothing downstream would miss. +func TestCreateOrModifyMicroflow_PreservesExportLevel(t *testing.T) { + const moduleID = model.ID("module-1") + stored := []*microflows.Microflow{{ + BaseElement: model.BaseElement{ID: "mf-api"}, + ContainerID: moduleID, + Name: "ACT_PublicApi", + ExportLevel: "API", + }} + ctx, written := microflowWriteProbe(t, stored, moduleID) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "ACT_PublicApi"}, + CreateOrModify: true, + } + if err := execCreateMicroflow(ctx, stmt); err != nil { + t.Fatalf("CREATE OR MODIFY MICROFLOW failed: %v", err) + } + if *written == nil { + t.Fatal("no microflow was written") + } + if got := (*written).ExportLevel; got != "API" { + t.Errorf("rewrite demoted the export level: got %q, want %q", got, "API") + } +} + +// TestDescribeMicroflow_ReportsUnauthorableProperties covers the read side of +// both carries. Neither the deep link nor a non-default export level has an MDL +// spelling, so DESCRIBE cannot emit them as re-executable text — but a +// describe -> rename -> exec COPY has nothing to preserve from, so staying +// silent would hand the reader output that looks complete and is not. +// +// The export-level line is conditional on purpose: every document in every +// module measured stores "Hidden", so emitting it unconditionally would add a +// comment to every describe in order to say nothing. +func TestDescribeMicroflow_ReportsUnauthorableProperties(t *testing.T) { + ctx, _ := newMockCtx(t) + name := ast.QualifiedName{Module: "MyModule", Name: "ACT_Item"} + + render := func(mf *microflows.Microflow) string { + return renderMicroflowMDL(ctx, "microflow", mf, name, nil, nil, nil) + } + + got := render(µflows.Microflow{Name: "ACT_Item", URL: "item/{Key}", ExportLevel: "API"}) + if !strings.Contains(got, "-- URL: item/{Key}") { + t.Errorf("describe omitted the deep link; the output reads as complete:\n%s", got) + } + if !strings.Contains(got, "-- Export level: API") { + t.Errorf("describe omitted a non-default export level:\n%s", got) + } + + // The control: an ordinary microflow gets neither line. Without this the + // test would pass against a describer that comments on every microflow. + plain := render(µflows.Microflow{Name: "ACT_Item", ExportLevel: "Hidden"}) + if strings.Contains(plain, "-- URL:") || strings.Contains(plain, "-- Export level:") { + t.Errorf("describe commented on defaults:\n%s", plain) + } +} + +// TestCreateOrModifyMicroflow_PreservesConcurrencySettings is the executor half, +// and the one that was actually reachable by a user: the backend already read +// AllowConcurrentExecution and MarkAsUsed back, but the rebuild in +// buildMicroflowFromStmt overwrote both with its own literals before the backend +// ever saw them. +// +// The direction is why this went unreported. The rebuild wrote `true`, so a +// microflow that DISALLOWED concurrent execution came back allowing it — the +// running app's concurrency protection removed. CE4899 fires on +// disallow-without-a-message, never on allow, so no checker says anything; the +// error message and its translations go at the same time. +func TestCreateOrModifyMicroflow_PreservesConcurrencySettings(t *testing.T) { + const moduleID = model.ID("module-1") + stored := []*microflows.Microflow{{ + BaseElement: model.BaseElement{ID: "mf-serial"}, + ContainerID: moduleID, + Name: "ACT_Serial", + AllowConcurrentExecution: false, + MarkAsUsed: true, + ConcurrencyErrorMessage: &model.Text{Translations: map[string]string{ + "en_US": "Already running", + "nl_NL": "Wordt al uitgevoerd", + }}, + ConcurrencyErrorMicroflow: "MyModule.ACT_OnBusy", + }} + ctx, written := microflowWriteProbe(t, stored, moduleID) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "ACT_Serial"}, + CreateOrModify: true, + } + if err := execCreateMicroflow(ctx, stmt); err != nil { + t.Fatalf("CREATE OR MODIFY MICROFLOW failed: %v", err) + } + if *written == nil { + t.Fatal("no microflow was written") + } + if (*written).AllowConcurrentExecution { + t.Error("rewrite re-allowed concurrent execution; the app's concurrency " + + "protection is gone and no checker reports it") + } + if !(*written).MarkAsUsed { + t.Error("rewrite cleared MarkAsUsed; the document is reported unused again") + } + if got := (*written).ConcurrencyErrorMicroflow; got != "MyModule.ACT_OnBusy" { + t.Errorf("rewrite dropped the concurrency error microflow: %q", got) + } + msg := (*written).ConcurrencyErrorMessage + if msg == nil || len(msg.Translations) != 2 { + t.Fatalf("rewrite dropped the concurrency error message (or its translations): %#v", msg) + } +} + +// TestCreateMicroflow_ConcurrencyDefaults is the control: a NEW microflow still +// gets Mendix's defaults. Carrying is only ever from a stored document, so the +// fix must not change what a create produces — allow concurrency, not marked as +// used, no error handling. +func TestCreateMicroflow_ConcurrencyDefaults(t *testing.T) { + const moduleID = model.ID("module-1") + ctx, written := microflowWriteProbe(t, nil, moduleID) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "ACT_Fresh"}, + } + if err := execCreateMicroflow(ctx, stmt); err != nil { + t.Fatalf("CREATE MICROFLOW failed: %v", err) + } + got := *written + if got == nil { + t.Fatal("no microflow was written") + } + if !got.AllowConcurrentExecution { + t.Error("a new microflow must default to allowing concurrent execution") + } + if got.MarkAsUsed { + t.Error("a new microflow must not be marked as used") + } + if got.ConcurrencyErrorMessage != nil || got.ConcurrencyErrorMicroflow != "" { + t.Errorf("a new microflow acquired concurrency error handling: %#v / %q", + got.ConcurrencyErrorMessage, got.ConcurrencyErrorMicroflow) + } +} diff --git a/mdl/visitor/visitor_catalog_test.go b/mdl/visitor/visitor_catalog_test.go index 8eba1f62b5..6603e2d895 100644 --- a/mdl/visitor/visitor_catalog_test.go +++ b/mdl/visitor/visitor_catalog_test.go @@ -80,6 +80,16 @@ func TestSelectFromCatalog(t *testing.T) { // change that splits it is caught here. {"scheduled events table", "SELECT * FROM CATALOG.SCHEDULED_EVENTS;"}, {"scheduled events with where", "SELECT Name FROM CATALOG.SCHEDULED_EVENTS WHERE Enabled = 1;"}, + // ENTITY_EVENT_HANDLERS starts with the ENTITY keyword but lexes as one + // IDENTIFIER (maximal munch), so it needs no catalogTableName entry — + // asserted so a lexer change that splits it is caught here rather than + // as the silent no-output QUEUES went through. + {"entity event handlers table", "SELECT * FROM CATALOG.ENTITY_EVENT_HANDLERS;"}, + // `Event` is an MDL lexer keyword (EVENT), and the column is named Event + // — so this pins the keyword reaching both a select list and a WHERE. + // The QUEUES precedent is why: a keyword the grammar does not expect + // parsed to NOTHING, with no error and no output. + {"entity event handlers with keyword column", "SELECT Microflow FROM CATALOG.ENTITY_EVENT_HANDLERS WHERE Moment = 'Before' AND Event = 'Commit';"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/modelsdk/mpr/testdata/v1-project/.mxcli/catalog.db b/modelsdk/mpr/testdata/v1-project/.mxcli/catalog.db new file mode 100644 index 0000000000..81b7c4d50b Binary files /dev/null and b/modelsdk/mpr/testdata/v1-project/.mxcli/catalog.db differ diff --git a/sdk/microflows/microflows.go b/sdk/microflows/microflows.go index 6f87f7fd8f..5f3497dcb4 100644 --- a/sdk/microflows/microflows.go +++ b/sdk/microflows/microflows.go @@ -30,6 +30,54 @@ type Microflow struct { // MarkAsUsed (#723 §A). ApplyEntityAccess bool `json:"applyEntityAccess"` + // ConcurrencyErrorMessage and ConcurrencyErrorMicroflow are what Mendix does + // when a second invocation arrives while one is already running and + // AllowConcurrentExecution is false — show this (translatable) message, or + // run this microflow. Mendix requires one of them in that case (CE4899). + // + // Neither has MDL syntax, and neither did AllowConcurrentExecution or + // MarkAsUsed, so the executor's rebuild wrote its own defaults over all + // four. The direction matters: the rebuild hardcoded `true`, so a microflow + // that DISALLOWED concurrent execution came back allowing it — the app's + // concurrency protection removed — and because "allow" needs no error + // message, CE4899 does not fire and nothing reports it. The error message + // and microflow went with it, translations included. + ConcurrencyErrorMessage *model.Text `json:"concurrencyErrorMessage,omitempty"` + ConcurrencyErrorMicroflow string `json:"concurrencyErrorMicroflow,omitempty"` + + // ExportLevel is Studio Pro's "Export level" — `Hidden` or `API`, the two + // members MicroflowsExportLevel declares. It decides whether the microflow + // is part of the module's public surface when the module is exported as a + // package, so losing it makes a protected module's API silently smaller. + // + // Carried, not authored: MDL has no syntax for it. Empty means "the stored + // document said nothing", and the writer defaults that to `Hidden` — never + // to the empty string, which is not a member of the enum. + // + // Measured across three real marketplace modules (Business Events 3.12.0, + // External Database Connector 6.2.3 and 6.3.0): every document of every + // type stores `Hidden`, because all three export at module level `Source`. + // So `Hidden` is the overwhelmingly common value and the right default — + // but it is a default, not the only value, and hardcoding it is what made + // this a drop rather than a no-op (#1120 follow-up). + ExportLevel string `json:"exportLevel,omitempty"` + + // URL is the microflow's deep link (Mendix 10.6+) — Studio Pro's "URL" + // field, e.g. `item/{Key}`. MDL has no syntax for it, so it is carried + // across a rewrite rather than authored. + // + // The fourth property in this struct to be lost the way #723 §A describes, + // after AllowConcurrentExecution, MarkAsUsed and ApplyEntityAccess: the + // writer hardcoded "" and this struct had no field, so every rewrite + // deleted the deep link. Nothing reports it — `mxcli check` and `mx check` + // both pass, because a microflow without a URL is perfectly valid; the loss + // is only visible in Studio Pro, which is how it reached a user (#1120). + URL string `json:"url,omitempty"` + // URLSearchParameters names the microflow parameters supplied as query-string + // arguments of the deep link, as qualified names. Stored beside URL and lost + // with it. + URLSearchParameters []string `json:"urlSearchParameters,omitempty"` + // Return type ReturnType DataType `json:"returnType,omitempty"` ReturnVariableName string `json:"returnVariableName,omitempty"` // Variable name for return value (e.g., "$Result") @@ -43,7 +91,10 @@ type Microflow struct { // Allowed module roles for execution AllowedModuleRoles []model.ID `json:"allowedModuleRoles,omitempty"` - // Concurrent execution settings + // Deprecated: never read and never written, and it does not describe what + // Mendix stores — there is no thread count in the model. The real + // concurrency state is AllowConcurrentExecution plus the two + // ConcurrencyError fields above. Kept only because the type is exported. ConcurrentExecutionSettings *ConcurrentExecutionSettings `json:"concurrentExecutionSettings,omitempty"` // Toolbox entries. A microflow can be exposed twice — once for the microflow @@ -468,6 +519,9 @@ type ActionActivity struct { } // ConcurrentExecutionSettings represents settings for concurrent execution. +// Deprecated: a fiction — nothing reads or writes it, and Mendix stores no +// thread count. See Microflow.AllowConcurrentExecution and its +// ConcurrencyErrorMessage / ConcurrencyErrorMicroflow siblings. type ConcurrentExecutionSettings struct { model.BaseElement Enabled bool `json:"enabled"` diff --git a/sdk/versions/mendix-10.yaml b/sdk/versions/mendix-10.yaml index 48e5ed6ee7..9700e4ee09 100644 --- a/sdk/versions/mendix-10.yaml +++ b/sdk/versions/mendix-10.yaml @@ -50,10 +50,9 @@ features: min_version: "10.0.0" mdl: "SYNCHRONIZE UNSYNCHRONIZED (nanoflow only)" show_page_with_params: - min_version: "11.0.0" - workaround: - description: "Pass data via a non-persistent entity or microflow parameter" - max_version: "10.99.99" + min_version: "10.0.0" + mdl: "SHOW PAGE Module.PageName WITH PARAMS ..." + notes: "Pages$PageSettings.parameterMappings introduced in 9.7.0, so every 10.x supports it" send_rest_request: min_version: "10.1.0" mdl: "SEND REST REQUEST ..." @@ -75,12 +74,15 @@ features: min_version: "10.0.0" mdl: "CREATE PAGE Module.Name (...) { ... }" page_parameters: - min_version: "11.0.0" - workaround: - description: "Use non-persistent entity or microflow parameter" - max_version: "10.99.99" + min_version: "10.0.0" + mdl: "CREATE PAGE Module.Name (Params: ($P: Module.Entity)) { ... }" + notes: "Pages$PageParameter introduced in 9.4.0, so every 10.x supports it. The + optional/default-value half (PageParameter.IsRequired, PageParameter.DefaultValue) + is 11.5.0 and MDL cannot express it; the writer omits both below 11.5." page_variables: - min_version: "11.0.0" + min_version: "10.20.0" + notes: "Pages$LocalVariable introduced in 10.17.0, but its DefaultValue in 10.20.0. + MDL always writes a default expression, so the floor is the later of the two." pluggable_widgets: min_version: "10.0.0" notes: "Widget templates are currently extracted from 11.6; may cause CE0463 on 10.x" @@ -199,9 +201,6 @@ deprecated: upgrade_opportunities: from_10_to_11: - - feature: "page_parameters" - description: "Replace non-persistent entity parameter passing with direct page parameters" - effort: "low" - feature: "design_properties_v3" description: "Atlas v3 design properties available for richer styling" effort: "low" diff --git a/sdk/versions/mendix-9.yaml b/sdk/versions/mendix-9.yaml index d45fe5ab07..103322d3da 100644 --- a/sdk/versions/mendix-9.yaml +++ b/sdk/versions/mendix-9.yaml @@ -27,11 +27,19 @@ features: synchronize_unsynchronized: min_version: "9.4.0" mdl: "SYNCHRONIZE UNSYNCHRONIZED (nanoflow only)" + show_page_with_params: + min_version: "9.7.0" + mdl: "SHOW PAGE Module.PageName WITH PARAMS ..." + notes: "Pages$PageSettings.parameterMappings introduced in 9.7.0 (mendixmodelsdk StructureVersionInfo)" pages: basic: min_version: "9.0.0" mdl: "CREATE PAGE Module.Name (...) { ... }" + page_parameters: + min_version: "9.4.0" + mdl: "CREATE PAGE Module.Name (Params: ($P: Module.Entity)) { ... }" + notes: "Pages$PageParameter and Page.parameters introduced in 9.4.0 (mendixmodelsdk StructureVersionInfo)" security: module_roles: diff --git a/sdk/versions/page_parameter_floor_test.go b/sdk/versions/page_parameter_floor_test.go new file mode 100644 index 0000000000..62bcf0badd --- /dev/null +++ b/sdk/versions/page_parameter_floor_test.go @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: Apache-2.0 + +package versions + +import "testing" + +// The page-parameter family's version floors, measured against the Mendix Model +// SDK's own StructureVersionInfo records (mendixmodelsdk 4.115.0, src/gen/pages.js): +// +// Pages$PageParameter / Page.parameters introduced 9.4.0 +// Pages$PageSettings.parameterMappings introduced 9.7.0 +// Pages$LocalVariable introduced 10.17.0 +// Pages$LocalVariable.defaultValue introduced 10.20.0 +// Pages$PageParameter.isRequired introduced 11.5.0 +// Pages$PageParameter.defaultValue introduced 11.5.0 +// +// The registry previously put all three features at 11.0.0, which refused +// `CREATE PAGE ... (Params: ...)` on every Mendix 10 project (mendixlabs/mxcli#1121). +// That figure matched the illustrative sample output in +// docs/11-proposals/PROPOSAL_version_aware_agent_support.md and nothing else. +// +// Each row has its floor and the version just below it, so a regression that +// re-raises a floor fails here rather than passing vacuously. +func TestPageParameterFamilyFloors(t *testing.T) { + r, err := Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + + tests := []struct { + area, name string + version SemVer + want bool + }{ + // Page parameters: 9.4.0. + {"pages", "page_parameters", SemVer{9, 3, 0}, false}, + {"pages", "page_parameters", SemVer{9, 4, 0}, true}, + {"pages", "page_parameters", SemVer{10, 0, 0}, true}, + {"pages", "page_parameters", SemVer{10, 24, 25}, true}, // the reported project + {"pages", "page_parameters", SemVer{11, 0, 0}, true}, + + // Passing an argument to one: 9.7.0. Without this a parameterised page + // is unreachable, which is the other half of #1121. + {"microflows", "show_page_with_params", SemVer{9, 6, 0}, false}, + {"microflows", "show_page_with_params", SemVer{9, 7, 0}, true}, + {"microflows", "show_page_with_params", SemVer{10, 24, 25}, true}, + + // Page variables: the element is 10.17.0 but its DefaultValue is 10.20.0, + // and MDL always writes a default expression, so the floor is 10.20.0. + {"pages", "page_variables", SemVer{10, 19, 0}, false}, + {"pages", "page_variables", SemVer{10, 20, 0}, true}, + {"pages", "page_variables", SemVer{11, 0, 0}, true}, + } + for _, tt := range tests { + if got := r.IsAvailable(tt.area, tt.name, tt.version); got != tt.want { + t.Errorf("IsAvailable(%s, %s, %v) = %v, want %v", + tt.area, tt.name, tt.version, got, tt.want) + } + } +} + +// A feature a 10.x project already has must not be advertised as something an +// upgrade to 11 would unlock. page_parameters was the headline row of that list. +func TestPageParametersNotAnUpgradeOpportunity(t *testing.T) { + r, err := Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + for _, f := range r.FeaturesAddedSince(SemVer{10, 24, 0}) { + if f.Area == "pages" && f.Name == "page_parameters" { + t.Errorf("page_parameters reported as added since 10.24 (min_version %v); "+ + "Mendix has had it since 9.4.0", f.MinVersion) + } + } +} diff --git a/sdk/versions/registry_test.go b/sdk/versions/registry_test.go index 69fda6a496..c6f693b3ae 100644 --- a/sdk/versions/registry_test.go +++ b/sdk/versions/registry_test.go @@ -79,8 +79,9 @@ func TestIsAvailable(t *testing.T) { {"domain_model", "view_entities", SemVer{11, 0, 0}, true}, // Basic entities available in 9.x+ {"domain_model", "entities", SemVer{9, 0, 0}, true}, - // Page parameters require 11.0+ - {"pages", "page_parameters", SemVer{10, 24, 0}, false}, + // Page parameters require 9.4+ (see page_parameter_floor_test.go) + {"pages", "page_parameters", SemVer{9, 3, 0}, false}, + {"pages", "page_parameters", SemVer{10, 24, 0}, true}, {"pages", "page_parameters", SemVer{11, 0, 0}, true}, // Unknown feature {"domain_model", "teleportation", SemVer{11, 0, 0}, false}, @@ -119,11 +120,11 @@ func TestFeaturesForVersion(t *testing.T) { t.Error("view_entities not found in features list") } - // Check that page_parameters is NOT available at 10.24 + // Check that design_properties_v3 is NOT available at 10.24 (Atlas v3 is 11.0+) for _, f := range features { - if f.Area == "pages" && f.Name == "page_parameters" { + if f.Area == "pages" && f.Name == "design_properties_v3" { if f.Available { - t.Error("page_parameters should NOT be available at 10.24") + t.Error("design_properties_v3 should NOT be available at 10.24") } } } @@ -140,16 +141,16 @@ func TestFeaturesAddedSince(t *testing.T) { t.Fatal("expected features added since 10.24.0, got none") } - // page_parameters (11.0+) should be in the list + // design_properties_v3 (11.0+) should be in the list found := false for _, f := range added { - if f.Name == "page_parameters" { + if f.Name == "design_properties_v3" { found = true break } } if !found { - t.Error("page_parameters should appear in features added since 10.24") + t.Error("design_properties_v3 should appear in features added since 10.24") } // entities (10.0+) should NOT be in the list