diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 4998ccfbf..85535a966 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -356,6 +356,8 @@ cases for these three BSON types — they fell to `default: return nil`. | An unquoted negative number in an XPath constraint fails to parse: `where [Amount > -7]` → `Parse error: extraneous input '7' expecting {',', ')'}`. Reported as "negative numeric literals truncate (`-7` becomes `-`)" | `xpathWord` — the name-part rule inside XPath — is a **negated token set** that did not exclude `MINUS`, so the sign was consumed as a name word and the digits were left stranded (hence the truncation appearance). The lexer deliberately keeps `-` out of `NUMBER_LITERAL` (a leading sign there mis-tokenises `$x -2`), leaving negation to the parser; the general grammar has `unaryExpression` for this and the XPath grammar simply never got the equivalent | `mdl/grammar/domains/MDLPage.g4` (`xpathValueExpr` gains `MINUS xpathValueExpr`; `MINUS` added to the `xpathWord` exclusion set), `mdl/visitor/visitor_xpath.go` (`buildXPathValueExpr`), `mdl/visitor/visitor_page_v3.go` (`xpathExprToString` emits `-7`, not `- 7`) | **The grammar fix alone is worse than the bug.** With the parser accepting `-7` but the XPath AST builder having no case for the new alternative, the constraint parses and silently serializes to `[Amount > ]` — a dropped operand instead of a loud parse error. Caught only because the visitor has a round-trip helper; the microflow write path uses `GetText()` and looked fine. **When adding a grammar alternative, check every consumer of that rule, not just the one your repro exercises.** **Scope correction**: the finding's own example (`addDays([%CurrentDateTime%], -7)`) still fails — `addDays` is a *microflow expression* function, not an XPath one, and it fails `CE0161` with a POSITIVE argument too, so the sign was never its problem. Repro `mdl-examples/bug-tests/it-18-xpath-negative-literal.mdl`; A/B on Mendix 11.12.1: pre-fix the script does not parse, fixed binary writes it and `mx check` reports 0 errors. issuetracker #18 | | Text painted by an **Atlas topbar widget is invisible in a dark theme** — the language selector measures ~1.13:1 contrast, glyph pixels spanning 4 luminance values out of 255. A theme override exists and *names the right element*, so it looks handled | Two separate mistakes stacked. (1) **Specificity**: Atlas's own rule is `.navbar-brand .widget-language-selector .current-language-text` at (0,3,0); a bare `.current-language-text` at (0,1,0) never wins, and only appears to on layouts that do not nest the selector under `.navbar-brand`. (2) **Wrong value**: `color: inherit` inherits *body ink*, which is dark, while the rail is dark in both palettes — so even at the winning specificity it measures 1.00:1. Atlas paints from `--bg-color-secondary` with a `#fff` fallback because it assumes a dark rail | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-atlas-map.scss` (the "Atlas fixups" block) | Re-declare **Atlas's own selector shape** so the weights match and source order decides, and resolve the colour through the rail token (`var(--mxt-rail-ink-active, var(--mxt-rail-ink))`) rather than `inherit`. List the bare and the `.navbar-brand`-nested selectors together — each is matched at its own specificity, so one rule covers both layouts. **Generalisable — the shape to look for**: a guard that names the right element is not evidence it applies. Read the *winning* declaration (`CSS.getMatchedStylesForNode` in DevTools, or the computed value) instead of the one you wrote. **Measure contrast, not colour**: reading `getComputedStyle(el).color` once and seeing a plausible value proves nothing — compute the WCAG ratio against the first non-transparent ancestor background, which is what turns "looks fine" into 1.13 vs 19.47. Reported from the RssReader test build; tests in `cmd/mxcli/theme/theme_test.go`, verified in a browser at 17.79:1 light / 19.47:1 dark | + +| A themed app is on-palette everywhere except a few widget details — the **Data Grid 2 pager caption is invisible** (1.02:1 on a dark ground), row-select checkboxes stay stock Mendix blue, popovers cast light-mode shadows. Re-pointing tokens changes nothing, and the same widget's other parts (the pager *buttons*) are fine | The theme source shipped by the **widget modules** (`themesource/datawidgets`, `atlas_web_content`) styles some things with Sass variables and literals — `datawidgets/web/variables.scss:18` is `$pagination-caption-color: #0a1325`. Sass resolves those at compile time, before any custom property exists, so the value is baked into `theme.compiled.css` and no `--mxt-*` can reach it. The parts that *do* work resolve `var(--gray-darker, …)` through Atlas: same bar, two mechanisms | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-widgets.scss` (the shared widget layer, imported after the theme partial) | Add a CSS rule per baked declaration, resolving through a token so both palettes follow. **The obvious fix does not work**: each module's `main.scss` imports `theme/web/custom-variables` before its own `!default` vars, so `$pagination-caption-color: var(--mxt-ink-muted)` there *would* win — but (1) the names collide with Atlas Core's, which feeds them to Sass colour functions (`atlas_core/web/_variables.scss:20` computes `mix($brand-primary, #e7e7e9, 10%)`; handing `mix()` a `var()` is a compile error) and (2) the worst offenders are not behind a variable at all — `_three-state-checkbox.scss` writes `#264ae5` and `rgba(#264ae5, 0.4)` directly. **Generalisable — the shape to look for**: read the **compiled CSS, not the SCSS**, when deciding what to override. The sources are full of `var(--token, #fallback)` declarations that already resolve correctly; in one measured app `#264ae5` appeared in 46 declarations and **24 were harmless fallbacks**, so grepping the source would have produced twice the rules for no benefit. Reported from the Formula1 test build (§33); verified in a browser: pager caption 1.02:1 → 6.99:1 console dark, 6.39:1 light, 6.78/5.93 on signal | | A loop's variable used after `end loop;` passes `mxcli check`, then `mx check` fails `[error] [CE0108] "Variable 'item' is defined but not in scope at this location."` at the referencing activity. Applies to the **iterator** and to anything the body introduces (a `retrieve`, a `$X = create …`, a call output) | Nothing tracked loop-variable *visibility*. MDL052 already covered the sibling rule — names are unique across the whole microflow (CE0111) — and the wording of that rule ("scoped to the WHOLE microflow") reads as if the variable is readable flow-wide. Uniqueness and visibility are different: the name is reserved everywhere, readable only inside the loop body | `mdl/executor/validate_microflow_loop_scope.go` (new `MDL053`, wired from `microflowValidator.validate`), skill `.claude/skills/mendix/write-microflows.md` | Map each loop-scoped name to the loop whose **own** body introduces it (nested loops keep their own names), then walk the flow with the set of enclosing loops and flag any reference from outside the owner. **A name claimed by two loops is dropped, not reported** — that is the MDL052/CE0111 case, and without the guard the MDL052 negative example started failing for the wrong reason: the first loop's own use of `$R` was blamed on the second loop's claim. **Generalisable**: a rule keyed by variable *name* needs an ambiguity escape hatch whenever another rule exists precisely because names can collide. Both flavours verified against mxbuild 11.12.1 (2 × CE0108 in one probe). Repro `mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl`; tests `mdl/executor/validate_microflow_loop_scope_test.go`. Found while working the sudoku findings, but **not** one of them — the numbered finding it was filed under is an app bug in that project, not an mxcli defect | | `mxcli oql` silently omits a whole column: a `select A, B …` renders only `A`, and the JSON output has no `B` key at all — no error, no empty column. Reproduces whenever the value of `B` is **null in the first row** | The runtime omits a null-valued column from a row's JSON object entirely, and `parseOQLFeedback` took the column list from `extractColumnOrder(rows[0])` — one row, chosen for its key *order*. Every later row was then projected onto that short list, so the column vanished from the result rather than showing NULLs | `cmd/mxcli/docker/oql.go` (`parseOQLFeedback`, `mergeColumnOrder`, `hasOnlyKnownKeys`) | Union the keys of **all** rows, inserting each new key directly after the last key already known rather than appending — merging `[A, C]` with `[A, B, C]` must give `[A, B, C]`, not `[A, C, B]`, or a column that is null early in the result set jumps to the end of the table. The per-row re-scan is skipped when a row carries no unseen key, so the uniform case still costs one length check. **Generalisable**: any "take the shape from the first element" over a sparse encoding is a silent-wrong-answer bug, not a formatting bug — the output looks complete. Tests `TestParseOQLFeedback_ColumnUnionAcrossRows`, `TestMergeColumnOrder`; proven by stubbing the union back to first-row-only and watching the reported symptom return. sudoku #39, first half | | `mxcli test` cannot run at all in a container without a Docker daemon — parsing, runner generation, model injection and the **entire mxbuild build** succeed natively, then `docker up` fails with "failed to connect to the docker API at unix:///var/run/docker.sock". So microflow tests are unavailable in exactly the environment mxcli targets (Claude Code web containers ship `/usr/bin/docker` with no daemon) | Only one step of the run was containerised — start the runtime and read its log — but it was wired directly to `docker compose`, with no seam for another way to run the app. `run --local` had all three pieces already (boot a standalone runtime, tee its log, restore) | `cmd/mxcli/docker/localapp.go` (new `StartLocalApp`), `cmd/mxcli/testrunner/runner_local.go` (new), `cmd/mxcli/testrunner/runner.go` (docker path extracted to `runDockerAndCapture`), `cmd/mxcli/cmd_test_run.go` + `main.go` (`--local`) | Give the run a seam — `runLocalAndCapture` / `runDockerAndCapture` — so both modes share parse/inject/parse-results/cleanup and differ only in how the app is started. **Two traps, both found by running it rather than reasoning about it.** (a) The runner reports via an **after-startup** microflow, so its LOG output happens DURING the start action, before the runtime's log subscriber attaches; registering the subscriber early is not possible (the runtime answers `LoggingException` pre-start). What actually carries the output is the JVM console tee, live from spawn — verified by A/B running with the early attach removed. (b) A **failing** test makes the runner return false, which makes the after-startup action fail, which makes `start` return an error — the first version reported that as a broken run and dumped a stack trace instead of the test report. A failed boot whose log shows a verdict is a normal outcome. Local runs use their own ports (8081/8091) and a `_test` database so a `run --local` dev loop can keep serving. Verified end-to-end in a daemon-less container: 2 passed; then 1 passed / 1 failed with exit 1, project restored. sudoku #41 | @@ -391,6 +393,12 @@ extracting `OffsetExpression`/`LimitExpression`. | A fresh clone of a project created by `mxcli new` goes dirty the first time anyone builds it: ~50 **tracked** files modified that nobody edited — every `javascriptsource/*/actions/*.js` gains a banner, `import { Big } from "big.js"` and `export async function`, plus the matching `javasource` stubs. In a cloud session with a stop-hook git check it reads as "uncommitted changes" at the end of clean work | The template ships the generated action stubs in a slightly older shape and MxBuild rewrites them all on the first build. `mx check` does **not** — only a build does — so nothing before the first `run --local` could reveal it, which is after the user has already committed | `cmd/mxcli/docker/settle.go` (`SettleGeneratedSources`), `cmd/mxcli/cmd_new.go` (step 5/6, `--skip-build`), `cmd/mxcli/init.go` (`/theme-cache/` in the generated ignore list) | Fix the *timing*, not the content: run the build while the project is still being created, so the settled form lands in the first commit. Do **not** reimplement the rewrite — it is mxbuild's generator and version-specific; run the real thing. Best-effort by contract (no JDK, no mxbuild, failed build → warning, never a failed creation), because a settled tree is a nicety and a usable project is the deliverable. The other half is gitignore: `theme-cache/` is a cache and says so. A/B on 11.12.1, both git-init'd then built: `--skip-build` → 50 dirty files, default → **0**. Tests `cmd/mxcli/docker/settle_test.go`. mxcli-todo #7 | | `mxcli check` reports `✓ Syntax OK` / `Check passed!`, then `mxcli exec` on the same script fails partway through with `failed to resolve page: page not found: Module.Page` — a button targeting a page the script creates further down. `exec` is not transactional, so the statements before the failure are already written to the .mpr | Page references are resolved in statement order at exec time. `check --references` already had an ordered pass (`validateForwardPageRefs`), but it needs `-p`; plain `check` had nothing, and plain `check` is what gets run | `mdl/executor/validate_page_order.go` (`ValidateScriptPageOrder`, MDL-PAGE01), wired in `cmd/mxcli/cmd_check.go` beside the other project-free validators | The soundness argument is what makes this work without a project: a **plain** CREATE later in the script would fail if the page already existed, so the script itself asserts the page does not exist yet and the earlier reference cannot resolve against the project either. `CREATE OR MODIFY`/`OR REPLACE` assert nothing, so they stay with `--references`, which can look. **Generalisable**: an ordering rule that seems to need project state often does not, once you find the statement that already asserts what you were going to look up. Two things the diagnostic must say and does: a cycle cannot be fixed by ordering (create one page without the linking widget, add it with `ALTER PAGE … INSERT`), and commit before executing a large script. Verified against all `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_page_order_test.go`, example `mdl-examples/bug-tests/todo-9-forward-page-reference.fail.mdl`. mxcli-todo #9 | | A sidebar navigation label renders truncated — "All tasks" shows as "All task" — under every theme (`signal`, `ledger`, `console`) and both variants, so switching theme does not help. Measured on the live DOM as `scrollWidth=56` inside `clientWidth=48` | Atlas's **closed** sidebar is an icon rail: `--navsidebar-width-closed: 48px` in Atlas's own `themesource/atlas_core/web/themes/_theme-default.scss`. The label is wider than the rail, and the closed rail expects an *icon*, not text. No mxcli theme sets any navigation width — the themes map colours — which is exactly why every theme reproduces it | `.claude/skills/mendix/theme-styling.md` (documented; no code change) | **A fix was written, verified, and reverted** — record the reason: adding `text-overflow: ellipsis` to the nav item helps only where Atlas also sets `white-space: nowrap`; elsewhere the label wraps to two readable lines, and the rule turns `All / tasks` into `All / t…`. Screenshotted both ways against the real compiled CSS. The answer belongs to the app (give nav items icons — what the rail is for — or keep the sidebar open), not to a theme that would impose it on every app. **Generalisable**: when a reported symptom traces to an upstream layout constant, reproduce the geometry against the real compiled CSS (48px container, real class names, Playwright measurement) — it takes minutes, tells you whose constant it is, and shows when the "obvious" CSS fix is a regression. mxcli-todo #19d | +| `create non-persistent entity X ( A: String not null error '…' )` (or `unique`) passes `mxcli check` AND `mxcli exec`, then the build fails **CE0070** "Validations rules are not allowed on entity 'X', because it is not persistable" | `not null` / `unique` ARE validation rules — Studio Pro models "required" and "uniqueness" as rules on the entity, not as column constraints — so Mendix rejects them on a non-persistable entity. Nothing in mxcli connected the attribute constraint to the entity's persistence kind | `mdl/executor/cmd_enumerations.go` (`validateNPEValidationRules`, called from `ValidateEntity`) | Add **MDL054**, error severity, fired from the CREATE path only. **Establish the construct matrix against mxbuild before writing the rule, not from the issue text** — verified on 11.6.6 that `not null` with a message, `not null` bare, AND `unique` each produce CE0070 while a plain attribute does not, so the bare form (easy to miss, since the reporter only showed the message form) is flagged too. The CREATE path is the only one that can run this: `ALTER ENTITY … ADD ATTRIBUTE` does not carry the persistence kind, the same limitation MDL020 has. Sweep `mdl-examples/` + `scripts/check-skill-mdl.sh` after adding any error-severity rule — a false positive there breaks every user with that shape. Negative test `mdl-examples/bug-tests/832-npe-validation-rules.fail.mdl` (`.fail.mdl` = must fail check, enforced by `make check-mdl`) plus `-ok.mdl` pinning the other edge; tests `TestValidateEntityNPEValidationRules`. Issue #832 | +| `retrieve $L from Mod.Entity where [Attr = $Var/Mod.Assoc/Attr]` passes `mxcli check` AND `mxcli exec`, then the build fails **CE0161** "Error(s) in XPath constraint" | Mendix XPath reaches at most ONE hop off a variable, and nothing checked the hop count | `mdl/executor/validate_microflow.go` (`checkXPathVariableTraversal`, `xpathVarTraversalRe`), called from the `*ast.RetrieveStmt` arm beside MDL047/MDL048 | Add **MDL055**, error severity, matching a `$var`-rooted path with **2+ segments**. **Establish the boundary against mxbuild first — it is narrower than it looks**: `$Var/Attr` VALID, `$Var/Mod.Assoc` VALID (one hop to the associated object), `$Var/Mod.Assoc/Attr` CE0161. A rule keying on "a module-qualified segment follows a variable" would reject the middle form, which is legal; key on hop count instead. Verified on 11.6.6 by building all three and dropping the offender to confirm the other two are clean. **Reject, don't try to serialize** — there is no valid XPath for the two-hop form, so the constraint must be restructured and only the author knows which way. **Verify the suggestion you emit**: both recommended rewrites (`retrieve $Related from $Var/Mod.Assoc;` then constrain on `$Related/Attr`; or invert to `[Mod.Assoc/Mod.Entity = $Var]`) were built and confirmed at 0 errors before the message claimed "both forms build clean". Negative test `mdl-examples/bug-tests/831-xpath-variable-traversal.fail.mdl` + `-ok.mdl`; test `TestXPathVariableTraversal`. Issue #831 | +| `mxcli check` reports a microflow rule (e.g. MDL048 `[id = $StringVar]`) but `mxcli exec` writes the same script silently — a script that skips `check` produces a project the build rejects | Two different validators. The exec path called `ValidateMicroflowBody` (semantic errors); the MDL0xx rule set lives in `ValidateMicroflow`, which was wired only into `cmd/mxcli/cmd_check.go` and the LSP. Same shape as #836, where a guard existed on every exec path but was never reached from validate | `mdl/executor/validate.go` (`validateMicroflowRules`, `execEnforcedMicroflowRules`) called from `mdl/executor/cmd_microflows_create.go` — the handler, mirroring `ValidateEntity` in `cmd_entities.go` (NOT `validateWithContext`, which would double-report under `check --references`) | Promote an **explicit allowlist** of rules verified against a real mxbuild, never the whole set. **Blanket promotion was tried and reverted**: it turns every error-severity rule into a write barrier, and `MDL009` ("enumeration splits require exactly one value per branch") was a **false positive** — a multi-value branch covering every enum value builds at 0 errors on 11.6.6, and the shipped `write-microflows` skill documents that very form (since retired, see the MDL056 row). It also broke an existing repo test whose fixture uses `else` on an enum split. `MDL008` by contrast IS correct (mxbuild: CE0079 per uncovered value + CE0773) — so before promoting any rule, build its construct and read the verdict. Warnings are never promoted. Tests `TestValidateMicroflowRules_ReachedFromExec` and `…_UnverifiedRulesNotPromoted` (the latter fails if the allowlist is widened carelessly). Issue #833 | +| `mxcli check` errors **MDL009** "enumeration splits require exactly one value per branch" on `when Open, Pending then` — but Mendix accepts it, so check rejects valid MDL and contradicts the shipped `write-microflows` skill | The rule asserted the opposite of the platform's behaviour. Nobody had built the construct: a multi-value branch covering every value **plus `(empty)`** builds at 0 errors on 11.6.6 | `mdl/executor/validate_microflow.go` (the `*ast.EnumSplitStmt` arm; `checkEnumSplitEmptyBranch`) | Retire the assertion and replace it with what actually fails: an enum split needs an outgoing flow per condition value, so a missing branch is **CE0079**. **MDL056** checks the `(empty)` branch — universal, verified to hold even on a `not null` enum attribute, so it needs no enumeration lookup and works from the statement alone. Full value coverage (the other half of CE0079) is deliberately NOT implemented: it needs the enum's member list, i.e. resolving the split variable's type against script or project, which `ValidateMicroflow` cannot see — guessing would trade one false positive for another. **Use a NEW rule ID rather than repurposing**, so anything citing the old number still means the old, wrong thing. `MDL008` (no `else`) is correct and stays — mxbuild gives CE0079 per uncovered value **plus** CE0773 on the else flow. Fix the skill in the same change: it documented the invalid `else` form. Tests `TestValidateMicroflow_EnumSplitMultipleValuesAllowed` / `…RequiresEmptyBranch`; repro `mdl-examples/bug-tests/mdl009-enum-split-empty-branch.fail.mdl` + `-ok.mdl` | +| A microflow using `split type` writes a project mxbuild cannot **load**: `KeyNotFoundException: The given key '' was not present in the dictionary` at `StreamingBsonUnitReader.ResolvePostponedProperties`. `mxcli check` ✓ and `mxcli exec` ✓; reproduced on 11.6.6 and 11.13.0 | Two gaps in the modelsdk writer, both the #791 shape. (1) `microflowObjectToGen` had no `*microflows.InheritanceSplit` case → `default: return nil`, so the split was dropped while three sequence flows kept pointing at its `$ID`. (2) `caseValueToGen` had no `InheritanceCase` case → every branch degraded to a bare `Microflows$NoCase`, losing the entity it selects on. Its value-receiver normalisation also omitted the type, so a pointer-only fix would still miss half the calls | `mdl/backend/modelsdk/microflow_write.go` (`microflowObjectToGen`, `caseValueToGen`) — mirror `sdk/mpr/writer_microflow.go` | Add both cases. **Diagnose with the #791 recipe**: `mxcli bson dump --type microflow`, collect every `$ID`, check each key ending in `Pointer` resolves (before: 27 objects / 3 dangling; after: 28 / 0). **Take field lists from the GENERATED type, not from legacy** — legacy writes `ErrorHandlingType` on the split but `initInheritanceSplit` has no such property, i.e. legacy writes a field Mendix does not define. **When adding a case-value type, update the value-receiver normalisation too.** Modelling rules confirmed on both versions while verifying: a type split needs an outgoing flow for every type INCLUDING the base (CE0090), and an `else` does NOT substitute for the base-type case. Tests `TestMicroflowRoundTrip_InheritanceSplit`, `TestCaseValueToGen_InheritanceCase{,ValueReceiver}`; repro `mdl-examples/bug-tests/split-type-dangling-pointer.mdl` | +| The `split type` docs and examples teach a shape that fails the build: `case Spec` + `else`, with no branch for the base entity → **CE0090** "The 'X' value should be configured for an outgoing flow". `mxcli check` passes, so the drift survived; `mdl-examples/bug-tests/365` and `475` both shipped it, and 475's own header claimed "mx check reports 0 errors" | `else` on an inheritance split serializes as `Microflows$NoCase` and IS accepted, so it looks like it covers the remainder — but it does not satisfy type coverage. The base entity needs its own `case` | `.claude/skills/mendix/write-microflows.md` (Type Split section) + `mdl-examples/bug-tests/365-…`, `475-…` | Cover EVERY type including the base; `else` is then redundant. Also give the split somewhere to go: branches converge on a merge continuing to the end event, so a non-void microflow needs a `return` after `end split;` (else MDL003 + **CE0067**). Matrix verified on 11.6.6 AND 11.13.0: `specs+base` 0 errors, `specs+base+else` 0 errors, `specs+else only` CE0090. **When repairing a bug-test fixture, preserve the scenario it pins** — 475 tests "exactly ONE non-split branch continues", so its added base case must TERMINATE; an empty (falling-through) body would make two branches continue and silently retire the regression. Confirmed after the edit that the post-split activity still renders outside both case bodies and the describe→exec roundtrip is mxbuild-clean. Known cosmetic artifact: DESCRIBE emits an empty `else` block that was never authored; it re-parses and builds clean | | A published OData service created purely from MDL passes `mxcli check` and then fails the build — `[CE0729] "The service name should not be empty."` and `[CE7375] "Attribute ID for entity 'X' must be published and be the key when associations are exposed as an associated object id."` — the second firing even with no associations exposed at all | Two defaults `CREATE ODATA SERVICE` never set. (1) `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties and only the first was set; the CONSUMED path had defaulted this for CE0339 all along. (2) `PublishAssociations` defaults to false = "associations as an associated object id", which Mendix only allows when the system `ID` is published as the key — but MDL's `expose (Attr (KEY))` publishes an ordinary attribute | `mdl/executor/cmd_odata.go` (`serviceName` fallback + heal on create-or-modify; `publishAssociationsFor`; `nonPersistablePublishedEntities` warning), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`PublishAssociationsSet`) | **Wider than reported**: the finding framed CE7375 as a non-persistable-entity problem. Measured on 11.12.1, the identical service with a PERSISTENT entity and a unique key builds 0 errors with `true` and CE7375 with `false` — so the default broke *every* published service, and non-persistable was just where it could not be worked around. Defaulting to true does not pick a preference; it picks the only value that can build from the MDL people write. Needs tri-state (`PublishAssociationsSet`) so an explicit `false` is still honoured, and `create or modify` no longer flips a stored value the author did not mention. **Why nothing caught it**: `mdl-examples/doctype-tests/10-odata-examples.mdl` sets both properties explicitly, so the repo's own example worked around both defaults. Tests `cmd_odata_service_name_test.go`, `cmd_odata_publish_associations_test.go`; examples `f1-10.1-…`, `f1-10.4-…`. mxcli-formula1 #10.1/#10.4 | | A typo in an OData property — `ReadMicroflow:` for `ReadMode:`, `ServiceNam:` for `ServiceName:` — passes `mxcli check` and `exec` reports success, but the model does not have the property. Hours can go into wondering why a published resource ignores its read microflow | The grammar accepts any `name: value` pair inside an OData property list, and the visitor's `switch` had no `default` — so an unrecognised name was dropped between parse and AST. The ALTER path has always answered `"unknown OData service property: %s"`; CREATE, PUBLISH ENTITY, the client and the external entity had nothing | `mdl/ast/ast_odata.go` (`UnknownProperties` on four statements), `mdl/visitor/visitor_odata.go` (four `default:` arms), `mdl/executor/validate_odata_properties.go` (`ValidateODataProperties`, MDL-ODATA01), wired in `cmd/mxcli/cmd_check.go` | The visitor is where the name is lost, so the visitor is where it must be recorded — a validator over the AST alone cannot see a key that was already discarded. Carry it as `UnknownProperties` and report at check time, before anything is written. The message names the property AND guesses the intended one (prefix/substring, then one edit), because a bare known-property list still leaves the reader diffing two spellings by eye. **Correction to the report**: `Pagesize:` is *not* silently dropped — the visitor lowercases before matching, so casing is never a typo, and the test pins that. Verified against every `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_odata_properties_test.go`. mxcli-formula1 suggested issue 8 | | A read-microflow-backed OData resource must declare a `System.ODataResponse` parameter and compute a count, even when the count is expensive (a full CSV scan) and nobody asked for it — with no MDL to say otherwise. Same for `$skip`/`$top` support | `Countable`, `SkipSupported` and `TopSupported` were written as literal `true` in the BSON writer's `ODataPublish$QueryOptions`; nothing above the writer could express them | `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`*bool` on `PublishedEntityDef`, `odataBoolPtr`), `model/types.go`, `mdl/executor/cmd_odata.go` (`astEntityDefToModel`), `mdl/backend/modelsdk/odata_write.go` (`boolOrDefault`), `odata_read_detail.go` (`falseOnly`) | Tri-state (`*bool`) is load-bearing: these default to **true**, so "unset" and "false" cannot share a representation or every existing script would silently turn them off. The reader maps a stored `true` back to nil (`falseOnly`) so DESCRIBE prints only what the author wrote instead of three defaults on every resource. Verified end to end on 11.12.1: `Countable: No` + a read microflow with **no** `$Response` parameter builds 0 errors, which is exactly the combination that was impossible before. Tests `cmd_odata_query_options_test.go`, `odata_write_test.go`. mxcli-formula1 #10.3 | @@ -401,4 +409,21 @@ extracting `OffsetExpression`/`LimitExpression`. | `ALTER MODULE X ADD JAR DEPENDENCY (…)` succeeds, `list jar dependencies` reports it, the build is **green** — and the runtime throws `SQLException: No JDBC driver found in app for URL`. `deployment/build.gradle` has no dependencies block and `find deployment -iname '**'` returns nothing | Not a bad write. Declaring and resolving are **separate steps**: the model records the coordinate, and `mx sync-java-dependencies ` is what downloads it into `vendorlib/`. Studio Pro runs that when you edit Module Settings; nothing headless was running it. Confirmed on 11.12.1 — a full `mxbuild --target=deploy` resolves nothing, and the sync command then fetches the jar | `cmd/mxcli/docker/javadeps.go` (`SyncJavaDependencies`, `UnvendoredJarDependencies`), `cmd/mxcli/cmd_sync_java_deps.go` (`mxcli sync-java-deps [--check]`), `cmd/mxcli/docker/runlocal.go` (vendors before boot), `mdl/executor/cmd_modules.go` (`warnUnvendoredJarDependencies`) | **How to find the missing step**: the reporter's open question was "does mxbuild skip Maven resolution, or does mxcli write it somewhere MxBuild cannot read?" — neither. `strings mx.dll | grep -i dependenc` surfaced `ISyncJavaDependenciesRunner`/`SkipManagedDependencySync`, and `mx --help` listed `sync-java-dependencies`. When a model-level write "works" but the artefact never appears, check whether the **toolset** has a separate command for it before suspecting the write. Wired at three levels so the gap cannot stay silent: the executor says so the moment it writes an unvendored coordinate, `run --local` resolves it before boot, and `--check` exits non-zero as a build gate. Resolution needs network, so every call site is best-effort with an actionable message. Tests `cmd/mxcli/docker/javadeps_test.go`. mxcli-formula1 #12 | | `$Total = 5;` does not parse — `no viable alternative at input '$Total=5'` — while `DECLARE $Total Integer = 0;` does, and so do `$X = HEAD($List)`, `$X = create M.E (…)` and `$X = execute database query …`. The error names the token, not the missing keyword | Assignment existed only as a **prefix** on specific activity statements (`(VARIABLE EQUALS)?` on CALL/CREATE/RETRIEVE/…), plus a `SET $Var = expression` statement. A plain value therefore required `SET`, which nothing in the error or the surrounding syntax suggested | `mdl/grammar/domains/MDLMicroflow.g4` (`setStatement : SET? …`), `cmd/mxcli/syntax/features_microflow.go` | Make the guessable form work rather than improve the error: `SET` is now optional and both spellings produce the same `MfSetStmt`/`ChangeVariableAction`. **Prove a grammar relaxation causes no regressions with a control binary, not by reading**: `git stash` the `.g4`, `make grammar`, build `bin/mxcli-control`, sweep every `mdl-examples/**/*.mdl` with both — 13 scripts fail, the *same* 13, all pre-existing. ANTLR's adaptive prediction picks the activity-prefixed alternatives over `setStatement` on its own; no ordering change was needed. Executed against a real .mpr, mxbuild reports 0 errors. Tests `mdl/visitor/visitor_microflow_bare_assign_test.go` (bare and keyword forms must agree on the AST, not merely both parse). mxcli-formula1 #13 | | `mxcli test tests/ -p app/App.mpr` fails with "no such file or directory" for a `tests/` that sits right next to the `.mpr` | Test paths resolved against the process CWD only. Defensible in isolation, but mxcli otherwise encourages naming the project (`-p`) rather than standing in its directory, and project auto-discovery searches outward — so the two conventions collide and the failure looks like a missing directory | `cmd/mxcli/cmd_test_run.go` (`resolveTestPaths`) | Fall back to project-relative **only when the CWD-relative path does not exist**: a `tests/` in both places must resolve to the one the user is standing in, since silently preferring the project's copy would run the wrong suite. A path that exists in neither is passed through unchanged so the error names what was typed, not a rewritten path the user never mentioned. Tests `cmd/mxcli/cmd_test_run_paths_test.go`. mxcli-formula1 #13 | +| A test annotated `@cleanup rollback` (or with no `@cleanup` at all — rollback is the documented default) still leaves its rows in the database; a misspelled strategy like `@cleanup rollbak` does the same, silently, while the run reports PASS | `TestCase.Cleanup` was parsed and then used nowhere. The after-startup runner had no seam to implement it — tests run inside the startup action, so there is no context the runner owns. The test endpoint creates that seam: it builds the `IContext` each test runs on | `cmd/mxcli/testrunner/endpoint.go` (the handler's execute block), `cmd/mxcli/testrunner/cleanup_strategy.go` | Wrap the call in `ctx.startTransaction()` … `ctx.rollbackTransaction()` in a **finally** (a throwing test is the one most likely to leave half-written data), gated on a `rollback=1` query parameter the client sends per test. Report `rolledBack`/`rollbackError` in the response and warn per test — a rollback that fails silently is worse than none. Reject an unknown `@cleanup` value at **parse** time so `--list` catches it too. Verify against the database, not the endpoint's own claim: run one test with rollback and one with `@cleanup none` in the same suite and query Postgres — the `none` row must be the only survivor | +| A suite passes under `mxcli test --attach` and fails under `--local`, with assertions that depend on startup state (a loaded cache, seeded reference data) seeing zero rows | The `--local` runner pointed after-startup at its own registration microflow and did **not** chain the project's own, so the app's startup logic never ran. It was a deliberate choice (a known baseline) but was invisible: the run printed only `After-startup set to MxTest.RegisterEndpoint`, never that the user's microflow had been displaced | `cmd/mxcli/testrunner/runner.go` (`runEndpoint`), `cmd/mxcli/testrunner/cleanup_strategy.go` (`describeStartup`) | Capture project state **before** generating the endpoint MDL, and pass `state.afterStartup` to `GenerateEndpointMDL` so the generated flow chains it — the hosted `--test-endpoint` path already did this, and the mismatch between the two was the bug. Add `--skip-app-startup` for a deterministic empty baseline, and always print which of the two happened. Note the startup microflow's writes run at boot, outside any test transaction, so `@cleanup rollback` does not undo them. mxcli-formula1 findings #19 | +| `mxcli test tests/ -p app/App.mpr --list` fails with `stat tests/: no such file or directory` while the same command without `--list` runs fine | The `--list` branch passed raw `args` to `ListTests`, bypassing `resolveTestPaths` — so a path relative to the project (rather than the working directory) resolved for execution but not for listing | `cmd/mxcli/cmd_test_run.go` (the `if list` branch) | Pass `resolveTestPaths(args, projectPath)` there too. When a command has two entry points into the same input, check both go through the same path resolution. mxcli-formula1 findings #15 | | After `SET` became optional, `mx check` on a project built from `02-microflow-examples.mdl` reports six errors that no MDL change caused: `[CE0109] "Undefined variable 'ProductList.Price'."` at four Aggregate list activities, and `[CE0015] "Aggregate function must specify a valid attribute."` at the expression-based one. Identical on both engines. `mxcli check` on the same script is silent | Two conversions existed for one syntax. `$Sum = sum($List.Price)` used to reach the dedicated `aggregateListStatement` rule; making `SET` optional put `setStatement` — alternative 5 of ~50 — in front of it, so ANTLR matched the lower-numbered alternative and the statement fell through to `buildSetStatement`'s fallback conversion, which joined list and attribute into one name and dropped the per-item expression entirely. Underneath, `buildListAggregateAsFunction` never appended the expression argument, so the SET path could not have seen it either | `mdl/grammar/domains/MDLMicroflow.g4` (`setStatement` moved LAST in `microflowStatement`), `mdl/visitor/visitor_microflow_statements.go` (`buildSetAggregate` replaces `extractVariableAndAttribute`), `mdl/visitor/visitor_microflow_expression.go` (`buildListAggregateAsFunction` appends the expression argument) | **A permissive alternative belongs last.** `$X = ` overlaps every `VARIABLE EQUALS ` statement in the rule — aggregates, list operations, RANGE — and ANTLR's ALL(*) picks the lowest-numbered alternative that matches, so a new general form silently steals from every specific one above it. **The measurement trap that let this ship**: the `SET?` change was swept with a control binary over every `mdl-examples/**/*.mdl` and found no difference — but with `mxcli check`, which parses and validates and never serializes. This defect lives between the AST and the BSON, where only `exec` + `mx check` can see it. Sweeping with `check` proves the grammar still *parses*; it proves nothing about what the visitor *builds*. For a grammar change, the control sweep must run the integration gate (`go test -tags integration -run TestMxCheck_DoctypeScripts`), not `check`. Fix proven by reverting both halves and watching the new tests fail with the reported symptom. Tests `mdl/visitor/visitor_microflow_aggregate_test.go`. Upstream CI on the ako→mendixlabs sync PR | +| `DESCRIBE microflow` prints a bare `else` on a `split type` the author never wrote one for, and each describe→exec pass accumulates another | An object-type decision always carries an `(empty)` outgoing flow (the null-object case), emitted by the builder whether or not an `else` was written. DESCRIBE rendered that flow as `else`. Invisible until the `InheritanceCase` writer landed — before that every branch degraded to `NoCase`, so nothing distinguished it from a real case | `mdl/executor/cmd_microflows_show_helpers.go` (the `elseFlow` block in the inheritance-split traversal) | Drop the `else` line when its body renders empty — the same `elseLineIdx`/truncate pattern the if/else emitters already use. Exec re-creates the flow, so the omission is lossless and the roundtrip is stable. **Do NOT 'fix' this in the builder**: removing the empty-entity branch there fails the build with **CE0089** "The '(empty)' value should be configured for an outgoing flow" — that flow is load-bearing and is why `else` cannot substitute for the base entity's case (CE0090); `(empty)` and the base type cover different things. That wrong fix was implemented first and caught only because every shape was re-run through mxbuild, not because a unit test failed. Tests `TestBuilder_InheritanceSplitKeepsEmptyCaseFlow` (builder must KEEP it) and `TestTraverseFlow_InheritanceSplitOmitsEmptyElse` (describe must not print it) | +| `alter page … set on ` reports `widget "X" not found` when the widget sits inside a datagrid column rendered as `customContent`; only CREATE OR REPLACE PAGE can touch it | `findInWidgetChildren`'s pluggable branch searched the grid's own `Object.Properties[].Value.Widgets` and matched columns by derived name, but never descended into a COLUMN's own content. Columns live at `Object.Properties[columns].Value.Objects[]`; their widgets are one level deeper at `Properties[content].Value.Widgets[]` | `mdl/backend/pagemutator/mutator.go` (`findInWidgetChildren`, the columns loop) | Descend into each column's `Properties[].Value` and reuse `findInWidgetArray(…, "Widgets", name)`. **Address by the nested widget's OWN name, not a `grid.column.widget` path**: DataGrid2 columns carry no stored name in the MPR (see `findBsonColumn`), so a column segment could only be a derived name that changes when the caption is edited — a path that goes silently stale. Keep the existing column-by-derived-name lookup working (a test pins it, or the descent could shadow it). Test `TestFindBsonWidget_InsideCustomContentColumn`; repro `mdl-examples/bug-tests/834-alter-customcontent-column-widget.mdl`. Issue #834 | +| `alter page … set Caption = '…' on ` fails with `widget has no Caption property` — for EVERY action button, nested or top-level | An ActionButton has no `Caption` document: its caption is a `Forms$ClientTemplate` under **`CaptionTemplate`** (Template → Items[] → Translation.Text), the same shape `setWidgetContentMut` already walked for `Content`. `setWidgetCaptionMut` only looked for `Caption` | `mdl/backend/pagemutator/mutator.go` (`setWidgetCaptionMut`, `setClientTemplateText`) | Fall back to `CaptionTemplate` via a shared `setClientTemplateText` helper, which `setWidgetContentMut` now uses too. **Found while fixing #834 — check whether a symptom reproduces OUTSIDE the reported context before attributing it**: this failed on a top-level button as well, so it was a second, independent defect and the #834 finder fix alone would not have made the reporter's command work. When a widget property will not set, dump the widget's key list (`mxcli bson dump --type page`) before assuming the setter is wired — the field is often stored under a different, template-shaped key | +| A published service will not build: every whole-number attribute is `[CE5016] "Attribute … has type Integer, but is published as Edm.Int32"`, and an exposed enumeration adds CE5016 plus `[CE4583] "Enumeration 'X' is not published in this service."` | `mendixAttrTypeToEdm` mapped Integer→Int32 (Mendix publishes it as **Int64**, same as Long), and the enum path wrote `Edm.String` while `EnumerationAsString` was hardcoded `false` — the one combination Mendix rejects, since with the flag false it wants the enumeration published as its own EDM enum type. The function's own comment flagged the unverified rows, and the existing unit test *pinned the wrong answer* | `mdl/executor/cmd_odata.go` (`mendixAttrTypeToEdm`, `enumPublishedAsString`, `publishedAttrType`), `model/types.go` (`PublishedMember.EnumerationAsString`), `mdl/backend/modelsdk/odata_write.go` + `sdk/mpr/writer_odata.go` (stop hardcoding the flag) | **Let mxbuild adjudicate the whole table at once**: publish one attribute of every Mendix type in one service and read the CE5016s off the build. That found Integer (reported) *and* Enumeration (only suspected), and confirmed String/Long/Decimal/Boolean/DateTime were already right — five verified rows for one build. Binary turns out to be unpublishable at all (CE5013), whatever type you give it. **A type and a flag that only work as a pair must travel as a pair** — `Edm.String` is ambiguous between String and a flattened enum, so the flag is the only thing distinguishing them and it belongs on the same struct. Watch for an existing test that encodes the bug: this one asserted `Edm.Int32`, so the fix *failed the suite* until the assertion was corrected. Tests `cmd_contract_test.go`, `cmd_odata_edm_type_test.go`. mxcli-formula1 #16 | +| `create or modify external entity Mod.E (… Countable: false)` — touching only an entity-level property — detonates every attribute: `[CE6612] "Attribute 'circuitId' of external entity 'Stg_Circuit' is not supported."`, one per attribute, leaving a project that cannot build | Not the executor: it already preserves attributes it was not asked to change (`if len(attrs) > 0`). One layer down, `attributeFromGen` handled `StoredValue` and `OqlViewValue` but **not** `Rest$ODataMappedValue`, so every attribute of an external entity read back with no `RemoteName`, and the writer's `isExternal && a.RemoteName != ""` arm then fell through to a plain StoredValue on the next read-modify-write | `mdl/backend/modelsdk/domainmodel.go` (`attributeFromGen` gains the `ODataMappedValue` / `ODataMappedPrimitiveCollectionValue` arms) | **The attribute-level half of #782**, which fixed the entity level and stopped there — when a read-modify-write loses data, check every *nesting level* of the read, not just the one named in the report. A polymorphic `Value` switch that silently ignores a variant is the shape to look for: it compiles, it reads, and it drops. Reproduce with a **local metadata file** (`MetadataUrl: './contract.xml'`) — no server needed, and the import is the same code path. Also learned here: the reported attribute *rename* (`name` → `Stg_Circuitname`) is a different thing entirely — it happens at import, from `reservedEntityAttrNames`, and `name` is **not** actually reserved (verified: Mendix builds an external entity with an attribute literally named `name`). Tests `external_entity_read_test.go`. mxcli-formula1 #25 | +| `execute database query … dynamic $Sql` reaches the runtime as the literal string `'$Sql'` — `Parser Error: syntax error at or near "$"` from the database, not from Mendix. Runtime-built SQL, and therefore query pushdown, is impossible | The builder quoted any dynamic query not already starting with a quote — right for `dynamic 'SELECT …'`, wrong for an expression — and the AST kept one `DynamicQuery` string whichever branch of the grammar produced it, so nothing downstream could tell them apart | `mdl/ast/ast_microflow.go` (`DynamicQueryIsExpression`), `mdl/visitor/visitor_microflow_actions.go` (set it in the `expr` branch), `mdl/executor/cmd_microflows_builder_calls.go` (`dynamicQueryExpression`) | **When a grammar has two alternatives that mean different things, the AST must record which one fired** — a shared field plus a "does it look quoted?" heuristic is a guess, and the workaround users find (`dynamic '' + $Sql`, which starts with a quote so the heuristic leaves it alone) is proof the heuristic is the bug. Verified by reading the stored BSON rather than by describe: `DynamicQuery\x00\x05\x00\x00\x00$Sql` — five bytes, no quotes. Tests `cmd_microflows_dynamic_query_test.go`. mxcli-formula1 #21 | +| `create odata client` against a service behind `authentication basic` prints `Warning: could not fetch $metadata: … HTTP 401`, creates the client anyway, and the following `create external entities from …` imports nothing — from a script that reports success | The statement's `HttpUsername`/`HttpPassword`/`HEADERS` are stored for the runtime, but the design-time fetch was a bare `client.Get`. The fetch failure is only a warning, so the empty client propagates silently | `mdl/executor/cmd_odata.go` (`metadataFetchAuth`, `metadataAuthFromStmt`, `fetchODataMetadata`), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`HttpUsernameIsLiteral` / `HeaderDef.ValueIsLiteral`) | **Only a literal is usable at design time.** The visitor strips a quoted literal's quotes, so `'f1api'` and `Module.ApiUser` both arrive as bare strings — the AST has to record which was written, or mxcli sends a *constant's name* as the password. Unresolved names are reported instead, which is also the honest answer: mxcli has no runtime to resolve a constant against. **A warning on a step something else silently depends on needs to say what breaks next** — the message now names the empty client and the import that will do nothing. Verified against a real basic-auth server that 401s without credentials and 403s without the custom header, so both had to arrive. Tests `cmd_odata_metadata_auth_test.go`. mxcli-formula1 #23 | +| Re-running `create or modify odata service` after editing a `publish entity` block changes nothing — the served `$metadata` is identical, and only `drop odata service` + create picks the edit up | The modify branch updated the service's scalar properties and never touched `EntityTypes` / `EntitySets` | `mdl/executor/cmd_odata.go` (modify branch rebuilds published entities via `astEntityDefToModel`, and carries `AllowedModuleRoles` through) | **Replace, don't merge**: a member removed from the script has to leave the service, which merging cannot express — the script is the description of the service. **Carry through what the statement cannot express**: role grants come from a separate `grant access on odata service` and would otherwise be dropped by a modify (reported; *not* reproduced on 11.12.1 — kept as a guard, and the commit says so rather than claiming a fix). Verified: same script yields `Label as 'label'` before and `Label as 'label' (Filterable, Sortable)` after, build stays at 0 errors. Tests `cmd_odata_modify_members_test.go`. mxcli-formula1 #26 | +| `create external entities from` a contract that restricts capabilities produces a project that will not build: `'Seasons' is marked Countable=False in the OData service, but True in the app`, `'latitude' is marked Filterable=False …` — one per restricted resource or property | Insert/Update/Delete restrictions were parsed; **Count/Filter/Sort were not**, so the import had nothing to honour and defaulted all three to true — on the one command whose entire job is fidelity to the contract | `mdl/types/edmx.go` (`EdmEntitySet.Countable`, `NonFilterableProperties`, `NonSortableProperties` + the three `applyCapabilityAnnotations` arms), `mdl/executor/cmd_contract.go` | An unannotated set still means countable/filterable/sortable — **silence in a contract is not a restriction**, it is OData's own default, so `nil` and `false` must stay distinguishable (`*bool`, as with the publish-side query options). The generated entity is compared against the contract at *build* time, so anything the contract can say is something the importer must be able to read. Tests `mdl/types/edmx_test.go`. mxcli-formula1 #24 | +| A contract property called `name` is generated as `Stg_Drivername` / `Circuitname` — prefixed with the remote type. A page written against the published `$metadata` then fails with `The selected attribute 'F1Live.Drivers.name' no longer exists`, and the *same* field carries a different name in every module because the remote type names differ | `attrNameForOData` disambiguates any name in `reservedEntityAttrNames`, and `name` was on that list with the comment "Mendix system-managed attribute for the object name". It is not: Mendix builds an external entity with an attribute literally named `name` | `mdl/executor/cmd_contract.go` (`reservedEntityAttrNames` loses one entry; the import now reports the renames it does make) | **Test the whole list at once, not the reported entry.** One contract with a property per listed name, prefixing disabled, then `mx check`: CE7247 "The name 'x' is a reserved word" for `id`/`owner`/`changedBy`/`changedDate`/`createdDate`/`type`/`context`, and silence for `name`. That turns "is the list wrong?" into "which rows are wrong?" for the cost of a single build, and it *earns* the seven entries that stay rather than leaving them as folklore. Two existing tests pinned the old behaviour and had to be corrected — a hand-maintained list of platform rules will accrete guesses unless each row can point at an error code. **Migration**: a re-import renames the attribute back, so references to the prefixed name must follow. Tests `cmd_contract_reserved_test.go`. mxcli-formula1 #28 | +| `MOVE JAVA ACTION …` / `MOVE ODATA SERVICE …` is a parse error (`no viable alternative at input 'MOVEJAVA'`), and neither `CREATE` form takes a folder clause — so those documents can never leave the module root from MDL | The `moveStatement` rule listed seven doctypes and nothing else; the missing ones were never unimplemented, just unlisted | `mdl/grammar/MDLParser.g4` (two alternatives), `mdl/ast/ast.go`, `mdl/visitor/visitor_entity.go` (dispatch **and** the MOVE FOLDER discriminator), `mdl/executor/cmd_move.go`, backend `MoveJavaAction` / `MovePublishedODataService`, `sdk/mpr` exports `MoveUnitByID` | Both reduce to the existing reparent primitive — a top-level document move is one containment row, so a new doctype is a list entry plus a lookup, not new machinery. **Watch the discriminator**: `MOVE FOLDER` is told apart from a document move by the *absence* of a doctype keyword, so every keyword added to the rule must also be added to that condition or a folder move starts parsing as a document move. **Verify placement by differential count, not by reading the model**: run the script with and without the MOVE lines and diff `select ContainmentName, count(*) from Unit` — three new Folders rows (a nested path creates two) and an unchanged Documents count says reparented rather than copied or dropped. Grepping blobs for names is a trap; stock modules are full of the same words. Tests `visitor_move_doctypes_test.go`, example in `18-folder-examples.mdl` — which must sit **before** that script's `drop module`, a mistake the integration gate caught and `mxcli check` did not. mxcli-formula1 #32 | +| `create odata client` with credentials given as constants (`HttpUsername: '@Module.ApiUser'`) still gets HTTP 401 and an empty client, after the fix that made literal credentials work. Sharpened by the same release making a constant `ServiceUrl` mandatory — the shape the tool insists on is the shape whose credentials it will not read | `resolveCredential` trusted the visitor's isLiteral flag. `'@Module.ApiUser'` **is** a STRING_LITERAL, so the flag said "literal" and the previous code sent the fifteen characters `@Module.ApiUser` as the username — and the unresolved-credential note did not fire either, because as far as the code knew nothing was unresolved | `mdl/executor/cmd_odata.go` (`resolveCredential`, `constantReference`, `designTimeConstants`) | **A syntactic classification is not a semantic one.** The visitor can say "this was a quoted string"; only the executor can say "this quoted string names a constant". Any flag of the form isLiteral needs the consumer to ask what the literal *contains* before treating it as a value. **The fix is to resolve, not to refuse**: a constant's design-time default is exactly what Studio Pro sends on the same fetch, so reading it is the value rather than a workaround — and mxcli already has the project open. Three spellings must all work (`'v'`, `@M.C`, `'@M.C'`); a dotted literal like a password containing a dot must not be mistaken for a reference. Tests `cmd_odata_metadata_auth_test.go`. mxcli-formula1 #23 follow-up | +| An app themed dark still shows light-mode drop shadows under the datagrid's filter-operator popover and dropdown filter lists | The generated widget layer re-pointed `.column-selectors` but not the four rules in `_datagrid-filters.scss` that bake the same two-layer shadow. Each already takes its *background* from `--bg-color-secondary`, so Atlas re-colours the panel and leaves the shadow — which is why it reads as a partial fix rather than an untouched widget | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-widgets.scss` | **Read the selectors out of the shipped `themesource/`, not the bug report** — the fourth here is `.dropdown-container .dropdown-list`, nested under a parent the report did not name. **Verify in the compiled CSS, never the source** (the §33 rule): apply the theme to a real project, run `mxbuild --target=deploy --java-home=… --java-exe-path=…`, then grep `theme-cache/web/theme.compiled.css` and check the *line number* — the fix must land after the widget module's own rule (30794 vs 27765 here) or the cascade eats it. A theme-cache file older than the SCSS you just wrote is a stale compile, and mtime is the cheapest way to catch it. mxcli-formula1 #33 / suggested issue 4 | +| A `MOVE` cannot be confirmed and a module's layout cannot be reviewed: `SHOW STRUCTURE` groups by document type at every depth and never names a folder, `DESCRIBE` answers for one document at a time, so checking where things ended up means opening the `.mpr` as SQLite | The read side of folders was simply never built. `MOVE`/`DROP FOLDER` write containment; nothing read it back | New `mdl/executor/cmd_list_folders.go` + grammar (`FOLDERS` lexer token, `showOrList FOLDERS (IN …)?` in `MDLCatalog.g4`, `FOLDERS` added to the `keyword` rule so it stays usable as an identifier), `mdl/ast/ast_query.go`, `mdl/visitor/visitor_query.go`, `mdl/executor/executor_query.go` | **A layout listing must show what is *not* there**: empty folders (`[0]`) and documents still at the module root, or it cannot be diffed against an intended layout — that is the whole use. Documents are indexed by `ContainerID` across every list call the backend offers, each best-effort, so a backend that cannot answer one kind yields a listing missing that kind rather than no listing. **Do not stub the hierarchy in the test** — `mkHierarchy` populates `moduleNames` but not `folderNames`, so `BuildFolderPath` returns `""` and every folder silently collapses into the module root, which looks exactly like the bug. Build it from the mock's `ListModules`/`ListUnits`/`ListFolders`, as `getHierarchy` does. Tests `cmd_list_folders_test.go`, example in `18-folder-examples.mdl`. mxcli-formula1 issue #2 | diff --git a/.claude/skills/mendix/manage-security.md b/.claude/skills/mendix/manage-security.md index 7236ffbc3..1fc8c9eae 100644 --- a/.claude/skills/mendix/manage-security.md +++ b/.claude/skills/mendix/manage-security.md @@ -95,6 +95,10 @@ create module role MyModule.Admin description 'Full administrative access'; create module role MyModule.User; create module role MyModule.Viewer description 'Read-only access'; +-- `or modify` updates an existing role's description instead of failing, so the +-- whole security script stays re-runnable rather than needing a run-once file. +create or modify module role MyModule.ApiUser description 'API consumer'; + -- Remove a module role drop module role MyModule.Viewer; ``` diff --git a/.claude/skills/mendix/organize-project.md b/.claude/skills/mendix/organize-project.md index a1a3008cf..64929037f 100644 --- a/.claude/skills/mendix/organize-project.md +++ b/.claude/skills/mendix/organize-project.md @@ -106,6 +106,47 @@ begin end; ``` +## Reading the Layout Back + +`list folders` shows the folder layout of a module and what is in each folder. +This is the counterpart to `move`: `move` puts a document somewhere, `list +folders` shows where everything actually is. + +```sql +-- One module +list folders in MyModule; + +-- Every module in the project +list folders; +``` + +``` +MyModule + (module root) [1] + Microflow ACT_Unfiled + Api [0] + Api/Published [1] + ODataService PublicApi + Support [1] + JavaAction Helper + +(3 folder(s), 3 document(s)) +``` + +Three things about the output are deliberate: + +- **Empty folders are listed** (`Api [0]`), so the listing is the whole layout + and can be diffed against an intended one. +- **Documents still at the module root** appear under `(module root)` — what is + not filed yet is the thing you most want to notice. +- **Ordering is stable**, so a diff between two runs shows only real movement. + +Use the CLI's `--json` flag for a row per document (`Module, Folder, Kind, Document`) +when comparing against a checked-in layout. + +Do **not** reach for `show structure` here: it groups by document type at every +depth and never shows which folder a document sits in. + ## Moving Documents The `move` command relocates existing documents between folders and modules. @@ -168,8 +209,17 @@ move page OldModule.CustomerPage to NewModule; | Nanoflow | `folder 'path'` (keyword) | `move nanoflow ...` | | Snippet | `folder: 'path'` (property) | `move snippet ...` | | Enumeration | N/A | `move enumeration ...` | +| Constant | N/A | `move constant ...` | +| Database connection | N/A | `move database connection ...` | +| Java action | N/A | `move java action ...` | +| OData service (published) | N/A | `move odata service ...` | | Entity | N/A | `move entity ...` (module only, no folders) | +**Java actions and published OData services have no folder clause on `create`**, so +`move` is the only way to place them — before this they were stuck at the module +root forever. Both are plain document units, so the move is model-level only: it +changes containment and nothing else. + **Note:** Pages and snippets use property syntax (`folder: 'path'` inside parentheses). Microflows and nanoflows use keyword syntax (`folder 'path'` before `begin`). Entities are embedded in domain models and can only be moved to a different module (no folder support). ## Example: Reorganize a Module @@ -243,3 +293,4 @@ drop folder 'Processing' in MyModule; - [ ] Cross-module moves: checked impact with `show impact of` first - [ ] Folder naming is consistent across modules - [ ] DROP FOLDER: verify folder is empty before dropping +- [ ] After a batch of moves: `list folders in MyModule` to confirm the layout diff --git a/.claude/skills/mendix/test-microflows.md b/.claude/skills/mendix/test-microflows.md index 8d1534bab..2a2f90ff8 100644 --- a/.claude/skills/mendix/test-microflows.md +++ b/.claude/skills/mendix/test-microflows.md @@ -93,6 +93,48 @@ The markdown format turns your tests into living documentation. | `@throws` | Expect error | `@throws 'validation failed'` | | `@cleanup` | Rollback strategy | `@cleanup rollback` (default) or `@cleanup none` | +### `@cleanup` — what happens to a test's data + +**`rollback` is the default**, so by default a test's database writes do not +survive it. The endpoint opens a transaction around the call and rolls it back +afterwards, including when the test throws. + +```mdl +/** + * @test creating an order does not leak + * @expect $result = 'ok' + */ +$result = CALL MICROFLOW Sales.CreateOrder(Amount = 100); +/ + +/** + * @test seed data the next test needs + * @cleanup none + */ +$result = CALL MICROFLOW Sales.SeedCatalogue(); +/ +``` + +Use `@cleanup none` when the writes are the point — seeding a fixture, or +inspecting the result in the running app afterwards. + +Two things worth knowing: + +- **`--local` only.** Rollback needs the test endpoint, which owns the context + the test runs in. The Docker / `--legacy-runner` path executes tests inside + the after-startup action and has no such seam, so it always commits. +- **A rollback that fails is reported, loudly.** The run prints a `WARNING` per + affected test and a summary line, because the alternative — data left behind + while the suite still says PASS — is the failure mode this annotation exists + to prevent. `--verbose` tags every test with `[rolled back]`, `[committed]` or + `[ROLLBACK FAILED]`. + +A misspelled strategy (`@cleanup rollbak`) is a **parse error**, not a silent +fallback to committing. + +Rollback matters most under `--attach`, where the database is the one your dev +app is using. + --- ## Running Tests @@ -130,8 +172,9 @@ older **after-startup microflow** pattern. 2. Records the project's current after-startup microflow, and whether an `MxTest` module already exists 3. Generates **one `MxTest.Test_` microflow per test**, plus a Java action - that registers an HTTP endpoint, and points after-startup at a microflow whose - only job is to call it — **no test runs during startup** + that registers an HTTP endpoint, and points after-startup at a microflow that + registers it and then **chains your own after-startup microflow** — + **no test runs during startup** 4. Builds and boots the app once 5. Invokes each test by name over HTTP; each returns its own verdict in the response @@ -150,6 +193,34 @@ Two consequences worth knowing when reading a failing run: Each test is a separate microflow with its own variable scope, so `$result` in one test never collides with `$result` in another. +#### Your app's after-startup microflow still runs + +The generated startup flow registers the endpoint and then calls the project's +own after-startup microflow, so tests see the app in the state it actually boots +into — a loaded cache, seeded reference data, whatever your app does. The run +says which happened: + +``` +After-startup set to MxTest.RegisterEndpoint (registers the endpoint; runs no tests, then runs your MyModule.ASU_Startup) +``` + +Pass `--skip-app-startup` when you want an empty, deterministic baseline +instead — the app seeds demo data and your tests assert on counts, say: + +``` +After-startup set to MxTest.RegisterEndpoint (… --skip-app-startup, so MyModule.ASU_Startup will NOT run) +``` + +This is why a suite behaves the same under `--local` and `--attach`. Before it +chained, `--local` ran with the app's startup logic suppressed, and a suite that +depended on startup state passed under `--attach` and failed under `--local` for +reasons unrelated to the code. + +One thing rollback does **not** cover: whatever the startup microflow writes +happens at boot, outside any test's transaction, so `@cleanup rollback` does not +undo it. Under `--local` that lands in the scratch `_test` database; +under `--attach` your app wrote it at its own boot regardless. + #### `--watch`: keep the runtime warm ```bash diff --git a/.claude/skills/mendix/theme-styling.md b/.claude/skills/mendix/theme-styling.md index 218fe8aff..51c778282 100644 --- a/.claude/skills/mendix/theme-styling.md +++ b/.claude/skills/mendix/theme-styling.md @@ -135,6 +135,43 @@ Keep the rail dark in both variants, or force `color: inherit` on those widgets. For a working implementation of all of the above, read the generated `theme/web/_mxcli-atlas-map.scss` in any themed project. +### Tokens stop at Atlas Core — the widget modules bake their colours + +Re-pointing Atlas's custom properties covers the app, and then a few things stay +stubbornly off-palette: the Data Grid 2 pager caption, row-select checkboxes, +popover shadows. One cause: the theme source shipped by the **widget modules** +(`themesource/datawidgets`, `atlas_web_content`) styles some things with Sass +variables and literals. Sass resolves those at compile time, before any custom +property exists, so the value is baked into `theme.compiled.css` and **no token +can move it**. Only a later CSS rule can. + +The worst case is `datawidgets/web/variables.scss:18`, +`$pagination-caption-color: #0a1325` — the "1–15 of 77" caption, which measured +**1.02:1** on a dark ground. The pager *buttons* beside it were fine, because +they resolve `var(--gray-darker, …)` through Atlas. Same bar, two mechanisms. + +**The obvious fix does not work.** Each module's `main.scss` imports +`theme/web/custom-variables` *before* its own `!default` variables, so setting +`$pagination-caption-color: var(--my-muted)` there would win and Sass would +substitute the `var()` into every use site. Tempting, and wrong here: + +1. The names collide with Atlas Core's, and Atlas Core feeds them to Sass colour + functions — `atlas_core/web/_variables.scss:20` computes + `mix($brand-primary, #e7e7e9, 10%)`. Handing `mix()` a `var()` is a compile + error, so the app stops building. +2. The worst offenders are not behind a variable at all: + `_three-state-checkbox.scss` writes `#264ae5` and `rgba(#264ae5, 0.4)` + directly, so overriding `$brand-primary` would not reach them. + +So it is a rule set, in a partial imported after the theme's own — see the +generated `theme/web/_mxcli-widgets.scss`. + +**Read the compiled CSS, not the SCSS, when building one.** The sources are full +of `var(--token, #fallback)` declarations that already resolve correctly; only +the bare literals are a problem. In one measured app the stock blue `#264ae5` +appeared in 46 declarations — **24 of them harmless fallbacks**. Grepping the +source would have produced twice the rules for no benefit. + ## CSS Hot-Reload Workflow For theme/styling changes during Docker development: diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index efdc57f08..326486134 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -421,31 +421,63 @@ Use `case` when a microflow branches on an enumeration value. case $Status when Open, Pending then return true; - when (empty) then + when Closed then return false; - else + when (empty) then return false; end case; ``` `(empty)` represents an unset enumeration value. Multiple values can share one `when` branch by separating them with commas. Case values are bare identifiers — do **not** quote them. +> **Every value needs a branch, including `(empty)` — and there is no `else`.** +> A Mendix enum split is an exclusive split with one outgoing flow per condition +> value, so an uncovered value fails the build with **CE0079** *"The 'X' condition +> value should be configured in properties for an outgoing flow."* `mxcli check` +> reports a missing `(empty)` branch as **MDL056**, and an `else` branch as +> **MDL008** (an `else` does not stand in for the missing flows: mxbuild reports +> CE0079 for each uncovered value *and* CE0773 on the else flow itself). +> +> The `(empty)` branch is required **even when the attribute is `not null`** — +> verified on Mendix 11.6.6. If several values share a path, put them in one +> branch (`when Open, Pending then`) rather than reaching for `else`. + ### Type Split And Cast Statements Use `split type` when a microflow branches on an object's runtime specialization. Use `cast` inside a type branch to create the specialized variable used by the branch body. ```mdl +declare $IsSpecialized boolean = false; split type $Input case Sample.SpecializedInput cast $SpecificInput; - return true; -else - return false; + set $IsSpecialized = true; +case Sample.BaseInput end split; +return $IsSpecialized; ``` -`case` values are qualified entity names. The optional `else` branch handles objects that do not match any listed specialization. +`case` values are qualified entity names. + +> **Every type needs a branch — including the base entity.** An object-type +> decision gets one outgoing flow per listed type, and a type with no flow fails +> the build with **CE0090** *"The 'X' value should be configured for an outgoing +> flow."* The base entity (the split variable's own type) counts: `case +> Sample.BaseInput` above is what covers "it is not any of the specializations". +> +> **`else` does not stand in for the base-type case.** It is accepted — it +> serializes as `Microflows$NoCase` — but it does not satisfy coverage, so +> `case Spec` + `else` still fails CE0090. Once every type has a branch, `else` +> is redundant. Verified on Mendix 11.6.6 and 11.13.0. +> +> **The split needs somewhere to go afterwards.** Branch bodies converge on a +> merge that continues to the microflow's end event, so a non-void microflow +> needs a `return` after `end split;` — otherwise `mxcli check` reports MDL003 +> and the build fails **CE0067** *"The 'Return value' property is required."* +> Doing the per-branch work into a variable and returning it once (above) is the +> clearest shape; returning inside every branch also works, but still needs the +> trailing `return`. **`cast` only stores the output variable.** Studio Pro persists Microflows$CastAction with a single `VariableName` field — the source variable is implicit (the type-split's input). Use `cast $SpecificName;` to give the specialized variable its name. The two-variable form `$Output = cast $Source;` parses but `$Source` is dropped on roundtrip; prefer the single-variable form. diff --git a/CLAUDE.md b/CLAUDE.md index 088cd61be..f39bf63e3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -610,7 +610,7 @@ Full syntax tables for all MDL statements (microflows, pages, security, navigati - LSP server with hover, go-to-definition, completion, diagnostics, symbols, folding - VS Code extension (`vscode-mdl`) with context menu commands (Run/Check/Selection) - Docker build integration (`mxcli docker build`) with PAD patching (Phase 1) -- Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`): local test runs go through a **token-guarded HTTP endpoint** registered by a generated Java custom request handler, instead of compiling the suite into the project's after-startup microflow. One microflow per test, resolved by name at request time from `Core.getMicroflowNames()` and invoked with `Core.microflowCall(...).execute(...)` — so a throwing test fails only itself (not the boot), results are returned rather than scraped from the runtime log, and each test has its own variable scope. The handler **survives `reload_model`** (after-startup does not re-run, the JVM is unchanged), which is what makes `--watch` possible: ~30s first run, then ~2s from an edit — to a test *or* to the microflow under test — to a verdict. `--attach` skips even that boot by running against an app already up under `run --local --test-endpoint`, driving that process's serve + admin APIs over loopback; it uses **that app's database**, only ever adds/removes its own test microflows, and refuses a change needing a restart. Security: the handler is **not registered at all** without `MXCLI_TEST_TOKEN` in the runtime env (so a project that kept the `MxTest` module through a failed cleanup is inert in production), the token is constant-time compared, non-loopback callers are refused, `/list` is clamped to the test namespace, and only `MxTest.Test_*` may be invoked. The token reaches the runtime via its environment and is never written into the project. Docker keeps the after-startup runner (`--legacy-runner` selects it locally). Packages: `cmd/mxcli/testrunner/` (`endpoint.go`, `client.go`, `watch.go`, `host.go`, `handshake.go`). See `docs/15-testing/SPIKE_test_endpoint_request_handler.md` +- Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`): local test runs go through a **token-guarded HTTP endpoint** registered by a generated Java custom request handler, instead of compiling the suite into the project's after-startup microflow. Boot registers the endpoint and then **chains the project's own after-startup microflow**, so tests see the app as it really boots (`--skip-app-startup` opts out) — without that, a suite depending on startup state passed under `--attach` and failed under `--local`. One microflow per test, resolved by name at request time from `Core.getMicroflowNames()` and invoked with `Core.microflowCall(...).execute(...)` — so a throwing test fails only itself (not the boot), results are returned rather than scraped from the runtime log, and each test has its own variable scope. Owning the `IContext` is also what finally makes **`@cleanup rollback`** (the annotation's documented default, previously parsed and ignored) real: the handler wraps the call in `startTransaction()`/`rollbackTransaction()`, so a test's writes do not survive it — verified against Postgres, with `@cleanup none` as the in-run control. A rollback that fails is reported per test and summarised, never silent; an unknown strategy is a parse error. The handler **survives `reload_model`** (after-startup does not re-run, the JVM is unchanged), which is what makes `--watch` possible: ~30s first run, then ~2s from an edit — to a test *or* to the microflow under test — to a verdict. `--attach` skips even that boot by running against an app already up under `run --local --test-endpoint`, driving that process's serve + admin APIs over loopback; it uses **that app's database**, only ever adds/removes its own test microflows, and refuses a change needing a restart. Security: the handler is **not registered at all** without `MXCLI_TEST_TOKEN` in the runtime env (so a project that kept the `MxTest` module through a failed cleanup is inert in production), the token is constant-time compared, non-loopback callers are refused, `/list` is clamped to the test namespace, and only `MxTest.Test_*` may be invoked. The token reaches the runtime via its environment and is never written into the project. Docker keeps the after-startup runner (`--legacy-runner` selects it locally). Packages: `cmd/mxcli/testrunner/` (`endpoint.go`, `client.go`, `watch.go`, `host.go`, `handshake.go`). See `docs/15-testing/SPIKE_test_endpoint_request_handler.md` - Warm local dev loop (`mxcli run --local [--watch] [--screenshot]`): Docker-free `mxbuild --serve` + standalone runtime, hot `reload_model` for behavioural changes and restart+DDL for structural ones (chosen from the serve build's `restartRequired`). Bundles the browser client (`web/dist/` via mxbuild's rollup runner, which the serve Deploy target skips) so Mendix 11.x apps render in a browser. `--watch` keeps an incremental rollup bundler hot (CHOKIDAR_USEPOLLING for container fs; ~3-4s page re-bundle, skipped for model-only edits) and watches only model source (`.mpr`+`mprcontents/`). `--ensure-db` provisions the local Postgres + app database if missing; `--setup` does the non-blocking prerequisites (cache mxbuild+runtime, ensure DB) and exits — `mxcli init` wires it into a Claude Code SessionStart hook so a fresh/reaped web session self-bootstraps, and `docs-site/src/tools/bootstrap-prompt.md` is the empty-repo seed prompt. `--screenshot` captures a Playwright PNG each change (pixel-perfect page loop), with `--screenshot-url` deep links (repeatable for multi-page sets, one PNG per page) and `--screenshot-user`/`--screenshot-password` form login (session saved as Playwright storage state, reused via `screenshot --load-storage`). See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` - External browser preview (`mxcli run --hub ` + `mxcli tunnel-hub`): the app stays local and reverse-tunnels out over a single 443 connection (embedded chisel) to a static relay, so it is reachable in a browser at a public URL — works from egress-only environments (Claude Code web), verified live through the session's MITM egress proxy. `run --hub` implies `--local`, boots the runtime with `ApplicationRootUrl` set to the assigned URL (so the SPA/`originURI` work under the public origin), resolves the control proxy honouring `NO_PROXY`, and retries forever. `mxcli tunnel-hub --domain ` is the **multi-tenant** relay: a registry keyed by prefix/project/solution/branch/worktree (stable URLs on reconnect) fronts many previews at per-subdomain hosts (`[prefix-]project[-branch].`; main collapses to the project) over one 443 with per-subdomain autocert, a registration API (`/api/register|status|deregister|backends|sessions`), and an availability overview at `hub./` **grouped by Claude Code session** (`/api/sessions`): each session lists the endpoints it exposed and links back to its `claude.ai/code` conversation. Client identity flags: `--hub-prefix`/`--hub-project`/`--hub-solution`/`--hub-branch`/`--hub-worktree` (project + branch auto-detected); `--hub-session` groups a session's endpoints (auto-detected from `CLAUDE_CODE_REMOTE_SESSION_ID`). Past sessions are retained: a durable per-session endpoint history (`--sessions-file`, default `~/.mxcli/hub-sessions.json`) survives restarts and reaping, and is pruned after `--session-retention` (default 30d) — so the overview shows offline sessions too (`SessionLog` in `cmd/mxcli/tunnelhub/sessions.go`). Package: `cmd/mxcli/tunnelhub/`. See `docs/11-proposals/PROPOSAL_mxcli_dev_warm_loop.md` (slices 3–4) - Tunnel-hub GitHub authentication (opt-in, gated on `--github-oauth-client-id`; absent = today's open hub): **viewer plane** — GitHub OAuth web flow + HMAC-signed SSO session cookie (`Domain=.`), owner-checked previews (`--require-auth` default on → 302 to login / 403 non-owner; soft mode filters the listing only), `/api/backends` filtered to the viewer (unauthenticated → 401), admin "signed in as" via `/api/whoami`. **Registration plane** — durable, hashed hub API keys (`--keys-file`, default `~/.mxcli/hub-keys.json`, survive restarts) presented as `X-Hub-Key` → stamps `Backend.Owner`; shared `X-Hub-Secret` still works as an owner-less fallback. **Key issuance** — the hub's `/cli` browser page mints a key from the session cookie (no PAT; the device flow was removed as Claude Code containers block GitHub's device endpoints), rotate-by-default + count + revoke-all; `mxcli auth hub login --token ` is the headless path; `run --hub` reads `MXCLI_HUB_KEY` (env → `~/.mxcli/auth.json`) and degrades to local-only if registration fails. Append-only JSONL audit trail (`--audit-log`, no secrets). Packages: `cmd/mxcli/tunnelhub/` (+`audit/`), `cmd/mxcli/hubauth/`. See `docs/11-proposals/PROPOSAL_hub_authentication.md` diff --git a/cmd/mxcli/cmd_test_run.go b/cmd/mxcli/cmd_test_run.go index 56f059b1c..46b7abcdd 100644 --- a/cmd/mxcli/cmd_test_run.go +++ b/cmd/mxcli/cmd_test_run.go @@ -38,14 +38,26 @@ loop can keep serving the same project while tests run. 1. Parses test files and extracts test blocks with @test/@expect annotations 2. Generates one microflow per test, plus a Java action that registers a token-guarded HTTP endpoint - 3. Boots the app once — startup only registers the endpoint, it runs no tests + 3. Boots the app once — startup registers the endpoint and then runs your own + after-startup microflow, so tests see the app as it really boots. No test + runs during startup 4. Invokes each test by name over HTTP; the verdict comes back in the response 5. Restores original project settings +Your after-startup microflow running is what makes a suite behave the same under +--local and --attach. Pass --skip-app-startup for an empty, deterministic +baseline instead — the run always prints which of the two it did. + Because each test is its own microflow invoked on its own, a test that throws fails only itself instead of ending the run, and results are returned rather than recovered from the runtime log. +It also makes @cleanup real. By default (@cleanup rollback) each test runs in a +transaction the endpoint rolls back afterwards, so its database writes do not +survive — use @cleanup none when the writes are the point. The Docker path +always commits: it runs tests inside the after-startup action and has no +context of its own to roll back. + The endpoint is only reachable from loopback, only with a per-run token passed to the runtime through its environment (never written into your project), and will only ever invoke the generated MxTest.Test_* microflows. With no token in @@ -121,6 +133,7 @@ Examples: legacyRunner, _ := cmd.Flags().GetBool("legacy-runner") watch, _ := cmd.Flags().GetBool("watch") attach, _ := cmd.Flags().GetBool("attach") + skipAppStartup, _ := cmd.Flags().GetBool("skip-app-startup") verbose, _ := cmd.Flags().GetBool("verbose") color, _ := cmd.Flags().GetBool("color") timeoutStr, _ := cmd.Flags().GetString("timeout") @@ -132,8 +145,10 @@ Examples: } if list { - // Just list tests, no execution needed - if err := testrunner.ListTests(args, os.Stdout); err != nil { + // resolveTestPaths here too: listing that cannot find a path execution + // finds is a confusing split, and `mxcli test tests/ -p app/App.mpr + // --list` hit exactly that (mxcli-formula1 findings #15). + if err := testrunner.ListTests(resolveTestPaths(args, projectPath), os.Stdout); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } @@ -147,19 +162,20 @@ Examples: } opts := testrunner.RunOptions{ - ProjectPath: projectPath, - TestFiles: resolveTestPaths(args, projectPath), - SkipBuild: skipBuild, - Local: local, - LegacyRunner: legacyRunner, - Watch: watch, - Attach: attach, - Timeout: timeout, - JUnitOutput: junitOutput, - Verbose: verbose, - Color: color, - Stdout: os.Stdout, - Stderr: os.Stderr, + ProjectPath: projectPath, + TestFiles: resolveTestPaths(args, projectPath), + SkipBuild: skipBuild, + Local: local, + LegacyRunner: legacyRunner, + Watch: watch, + Attach: attach, + SkipAppStartup: skipAppStartup, + Timeout: timeout, + JUnitOutput: junitOutput, + Verbose: verbose, + Color: color, + Stdout: os.Stdout, + Stderr: os.Stderr, } result, err := testrunner.Run(opts) diff --git a/cmd/mxcli/lsp_completions_gen.go b/cmd/mxcli/lsp_completions_gen.go index 40b294377..00b1f9edd 100644 --- a/cmd/mxcli/lsp_completions_gen.go +++ b/cmd/mxcli/lsp_completions_gen.go @@ -256,6 +256,7 @@ var mdlGeneratedKeywords = []protocol.CompletionItem{ {Label: "AUTOFILL", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, {Label: "URL", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, {Label: "FOLDER", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, + {Label: "FOLDERS", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, {Label: "PASSING", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, {Label: "CONTEXT", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, {Label: "EDITABLE", Kind: protocol.CompletionItemKindKeyword, Detail: "Widget keyword"}, diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index be87beadd..c6fc84421 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -373,6 +373,7 @@ func init() { testRunCmd.Flags().Bool("local", false, "Run on mxcli's local runtime instead of Docker (no daemon needed)") testRunCmd.Flags().Bool("legacy-runner", false, "With --local, run tests from the after-startup microflow and parse the log, instead of over the test endpoint") testRunCmd.Flags().BoolP("watch", "w", false, "With --local, keep the runtime warm and re-run the suite on every test or model change (Ctrl-C to stop)") + testRunCmd.Flags().Bool("skip-app-startup", false, "With --local, do not run the project's own after-startup microflow during the test run (it runs by default, so tests see the app as it really boots)") testRunCmd.Flags().Bool("attach", false, "Run against an app already started with 'mxcli run --local --test-endpoint' instead of booting one (tests hit that app's database)") testRunCmd.Flags().BoolP("verbose", "v", false, "Show all runtime log output") testRunCmd.Flags().BoolP("color", "", false, "Use colored output") diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 85560be1e..a2d8e5ed1 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -211,6 +211,8 @@ SHOW STRUCTURE DEPTH 1 ALL;`, "move folder", "drop folder", }, Syntax: `MOVE Module.Name TO FOLDER 'Path'; +-- doctype: PAGE | MICROFLOW | NANOFLOW | SNIPPET | ENUMERATION | CONSTANT +-- | DATABASE CONNECTION | JAVA ACTION | ODATA SERVICE | ENTITY | FOLDER MOVE Module.Name TO TargetModule; MOVE OldModule.Name TO FOLDER 'Path' IN NewModule; MOVE FOLDER Module.FolderName TO FOLDER 'Path'; @@ -224,12 +226,50 @@ MOVE MICROFLOW MyModule.ACT_ProcessOrder TO FOLDER 'Orders/Processing'; -- Move entity to different module MOVE ENTITY OldModule.Customer TO NewModule; +-- Java actions and published OData services have no folder clause on CREATE, +-- so MOVE is the only way to place them +MOVE JAVA ACTION MyModule.ODataQuery TO FOLDER 'Support'; +MOVE ODATA SERVICE MyModule.PublicApi TO FOLDER 'Api/Published'; + -- Check impact before cross-module move SHOW IMPACT OF OldModule.CustomerPage; MOVE PAGE OldModule.CustomerPage TO NewModule; -- Drop empty folder -DROP FOLDER 'OldFolder' IN Module;`, +DROP FOLDER 'OldFolder' IN Module; + +-- Read the placement back +LIST FOLDERS IN MyModule;`, + SeeAlso: []string{"folders"}, + }) + + // ── Folders ───────────────────────────────────────────────────────── + + Register(SyntaxFeature{ + Path: "folders", + Summary: "LIST FOLDERS — the folder layout of a module, with what is in each folder", + Keywords: []string{ + "folders", "list folders", "show folders", "layout", + "folder tree", "where is this document", "unfiled", + }, + Syntax: "LIST FOLDERS [IN ];", + Example: `-- Layout of one module +LIST FOLDERS IN MyModule; + +-- Every module in the project +LIST FOLDERS; + +-- As rows, to diff against an intended layout +mxcli -p app.mpr --json -c "LIST FOLDERS IN MyModule" + +-- Complements MOVE: MOVE places a document in a folder, LIST FOLDERS reads +-- the placement back. SHOW STRUCTURE is organised by document type at every +-- depth, so it never shows which folder a document sits in. +-- +-- Empty folders are listed too (with [0]), and documents still at the module +-- root appear under "(module root)" — so the output is the whole layout and +-- can be diffed against an intended one.`, + SeeAlso: []string{"move", "structure"}, }) // ── Search ────────────────────────────────────────────────────────── @@ -283,6 +323,9 @@ Flags: every test or model change (Ctrl-C to stop) --attach Run against an app already started with 'mxcli run --local --test-endpoint' — no boot at all + --skip-app-startup + With --local, do not run the project's own + after-startup microflow (it runs by default) --legacy-runner With --local: use the old after-startup runner -v, --verbose Show runtime log lines -t, --timeout DUR Runtime startup timeout (default: 5m) @@ -292,13 +335,23 @@ Annotations: @expect $var = value Assert variable equals value @expect $obj/Attr = val Assert entity attribute @throws 'message' Expect error - @cleanup rollback|none Cleanup strategy (default: rollback) + @cleanup rollback|none What happens to the test's database writes. + rollback (the default) wraps the test in a + transaction and rolls it back, so nothing it + wrote survives — including when it throws. + none lets the writes commit. --local only: + the Docker path always commits. An unknown + value is a parse error, not a silent commit. How --local runs tests: one microflow per test, invoked by name over a token-guarded HTTP endpoint the app registers at boot. A test that throws fails only itself, and results are returned rather than scraped from the log. Docker still uses the older after-startup runner. +Boot also runs the project's own after-startup microflow, chained after the +endpoint registration, so tests see the app in the state it really boots into +and a suite behaves the same under --local and --attach. + Cost of a run: cold (--local) ~30s boots a runtime on its own ports + DB warm (--local --watch) ~2s runtime stays up between runs diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 9f6eba6f5..7481baf8b 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -21,8 +21,8 @@ func init() { Keywords: []string{ "module role", "create role", "drop role", }, - Syntax: "CREATE MODULE ROLE . [DESCRIPTION ''];\nDROP MODULE ROLE .;", - Example: "CREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\nCREATE MODULE ROLE Shop.User DESCRIPTION 'Read-only access';", + Syntax: "CREATE [OR MODIFY] MODULE ROLE . [DESCRIPTION ''];\nDROP MODULE ROLE .;", + Example: "CREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\n-- OR MODIFY makes a security script re-runnable:\nCREATE OR MODIFY MODULE ROLE Shop.User DESCRIPTION 'Read-only access';", SeeAlso: []string{"security.user-role", "security.entity-access"}, }) diff --git a/cmd/mxcli/testrunner/cleanup_strategy.go b/cmd/mxcli/testrunner/cleanup_strategy.go new file mode 100644 index 000000000..66c855ba4 --- /dev/null +++ b/cmd/mxcli/testrunner/cleanup_strategy.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "fmt" + "io" + "sort" + "strings" +) + +// Cleanup strategies for the @cleanup annotation. +// +// Rollback is the default and always was — the annotation has documented it +// since the runner shipped. It only became real with the test endpoint: the +// endpoint owns the context each test runs in, so it can open a transaction +// around the call and roll it back. The after-startup runner has no such seam, +// which is why the annotation sat parsed-but-unused. +const ( + // CleanupRollback wraps the test in a transaction and rolls it back, so its + // database writes do not survive. The default. + CleanupRollback = "rollback" + // CleanupNone lets the test's writes commit and persist. + CleanupNone = "none" +) + +// cleanupStrategies is the set of accepted @cleanup values. +var cleanupStrategies = map[string]string{ + CleanupRollback: "wrap the test in a transaction and roll it back (default)", + CleanupNone: "let the test's writes commit and persist", +} + +// validateCleanup rejects an unrecognised @cleanup value. +// +// Silently treating a typo as "not rollback" is the worst outcome available: +// `@cleanup rollbak` would leave the test's data in the database while the run +// still reported a clean pass, and nothing anywhere would say why. An unknown +// value is a mistake in the test file, so it is an error. +func validateCleanup(value string) error { + if value == "" || cleanupStrategies[value] != "" { + return nil + } + valid := make([]string, 0, len(cleanupStrategies)) + for k := range cleanupStrategies { + valid = append(valid, k) + } + sort.Strings(valid) + return fmt.Errorf("unknown @cleanup strategy %q (expected one of: %s)", value, strings.Join(valid, ", ")) +} + +// rollsBack reports whether a test's writes should be rolled back. +// +// An empty strategy means the annotation was absent, which is the default — +// rollback. Anything unrecognised has already been rejected by validateCleanup, +// so this never has to guess. +func rollsBack(tc TestCase) bool { + return tc.Cleanup == "" || tc.Cleanup == CleanupRollback +} + +// reportRollbackFailure explains why a requested rollback did not happen. +// +// Two causes are worth telling apart. The endpoint may not support rollback at +// all — with --attach the app is hosted by whatever mxcli started it, which can +// predate this feature — and that is a different fix from a transaction the +// runtime refused to roll back. +func reportRollbackFailure(w io.Writer, tc TestCase, rr *runResponse) { + switch { + case !rr.RollbackRequested: + fmt.Fprintf(w, " WARNING: %s ran without rollback — the app is hosting an older test endpoint\n"+ + " that ignores it. Restart the hosting 'mxcli run --local --test-endpoint'.\n", tc.Name) + case rr.RollbackError != "": + fmt.Fprintf(w, " WARNING: %s could not be rolled back: %s\n", tc.Name, rr.RollbackError) + default: + fmt.Fprintf(w, " WARNING: %s could not be rolled back (no reason reported)\n", tc.Name) + } +} + +// rollbackNote annotates a verbose result line with what happened to the +// transaction. +func rollbackNote(requested bool, rr *runResponse) string { + switch { + case !requested: + return " [committed]" + case rr.RolledBack: + return " [rolled back]" + default: + return " [ROLLBACK FAILED]" + } +} + +// describeStartup says what the generated after-startup microflow will do, +// naming the project's own microflow when there is one. +// +// This line exists because its absence was a reported trap (mxcli-formula1 +// findings #19). The runner printed only that after-startup had been pointed at +// its own microflow; a reader had no way to tell that their app's startup logic +// — a cache load, in that report — was therefore not going to run. The suite +// passed under --attach, where the app boots normally, and failed under --local +// for reasons that had nothing to do with the code under test. +func describeStartup(appAfterStartup string, skipped bool) string { + base := "After-startup set to " + endpointStartupFlow + " (registers the endpoint; runs no tests" + switch { + case appAfterStartup == "": + return base + "; this project has no after-startup microflow of its own)" + case skipped: + return base + "; --skip-app-startup, so " + appAfterStartup + " will NOT run)" + default: + return base + ", then runs your " + appAfterStartup + ")" + } +} diff --git a/cmd/mxcli/testrunner/cleanup_strategy_test.go b/cmd/mxcli/testrunner/cleanup_strategy_test.go new file mode 100644 index 000000000..ea4e4c35e --- /dev/null +++ b/cmd/mxcli/testrunner/cleanup_strategy_test.go @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRollsBack(t *testing.T) { + tests := []struct { + name string + cleanup string + want bool + }{ + {"absent annotation defaults to rollback", "", true}, + {"explicit rollback", CleanupRollback, true}, + {"explicit none", CleanupNone, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rollsBack(TestCase{Cleanup: tt.cleanup}); got != tt.want { + t.Errorf("rollsBack(%q) = %v, want %v", tt.cleanup, got, tt.want) + } + }) + } +} + +// TestRollbackIsTheDefault pins the contract the annotation has always +// documented: a test with no @cleanup rolls back. It went unimplemented until +// the endpoint gave the runner a context of its own to open a transaction on. +func TestRollbackIsTheDefault(t *testing.T) { + if !rollsBack(TestCase{}) { + t.Error("a test with no @cleanup annotation does not roll back") + } +} + +func TestValidateCleanup(t *testing.T) { + for _, ok := range []string{"", CleanupRollback, CleanupNone} { + if err := validateCleanup(ok); err != nil { + t.Errorf("validateCleanup(%q) rejected a valid strategy: %v", ok, err) + } + } +} + +// TestValidateCleanupRejectsATypo pins the reason this validation exists at all. +// Treating an unrecognised value as "not rollback" would leave the test's data +// in the database while the run still reported a clean pass — the worst +// available outcome, because nothing anywhere would say why. +func TestValidateCleanupRejectsATypo(t *testing.T) { + err := validateCleanup("rollbak") + if err == nil { + t.Fatal("a misspelled strategy was accepted; it would silently skip the rollback") + } + if !strings.Contains(err.Error(), "rollbak") { + t.Errorf("error %q does not quote the offending value", err) + } + // The message has to say what IS allowed, or the user is left guessing. + for _, want := range []string{CleanupRollback, CleanupNone} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not list the valid strategy %q", err, want) + } + } +} + +// TestParserRejectsABadCleanup pins that the rejection happens at parse time, so +// --list catches it too and no runtime is booted for a test file that cannot be +// run correctly. +func TestParserRejectsABadCleanup(t *testing.T) { + body := `/** + * @test something + * @cleanup rollbak + */ +$r = CALL MICROFLOW Mod.A(); +/ +` + dir := t.TempDir() + path := filepath.Join(dir, "bad.test.mdl") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + if _, err := ParseTestFile(path); err == nil { + t.Fatal("a .test.mdl with a misspelled @cleanup parsed without error") + } +} + +// TestMarkdownParserRejectsABadCleanup covers the other file format — the two +// parsers are separate code paths and the first version of this validation only +// reached one of them. +func TestMarkdownParserRejectsABadCleanup(t *testing.T) { + body := "```mdl-test\n/**\n * @test something\n * @cleanup rollbak\n */\n$r = CALL MICROFLOW Mod.A();\n```\n" + dir := t.TempDir() + path := filepath.Join(dir, "bad.test.md") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + if _, err := ParseTestFile(path); err == nil { + t.Fatal("a .test.md with a misspelled @cleanup parsed without error") + } +} + +func TestEndpointJavaSupportsRollback(t *testing.T) { + for _, want := range []string{ + `"1".equals(request.getParameter("rollback"))`, + "ctx.startTransaction();", + "ctx.rollbackTransaction();", + } { + if !strings.Contains(endpointJava, want) { + t.Errorf("the handler is missing %q", want) + } + } +} + +// TestEndpointRollbackIsInAFinallyBlock pins that a test which throws still gets +// its transaction rolled back. Without the finally, a failing test would be +// exactly the one that leaves its half-written data behind. +func TestEndpointRollbackIsInAFinallyBlock(t *testing.T) { + execute := strings.Index(endpointJava, "Core.microflowCall(mf).execute") + finallyIdx := strings.Index(endpointJava, "} finally {") + rollbackIdx := strings.Index(endpointJava, "ctx.rollbackTransaction();") + + if finallyIdx < 0 { + t.Fatal("the execution is not wrapped in try/finally") + } + if !(execute < finallyIdx && finallyIdx < rollbackIdx) { + t.Errorf("the rollback is not in the finally block after execution (execute=%d finally=%d rollback=%d)", + execute, finallyIdx, rollbackIdx) + } +} + +// TestEndpointStartsTheTransactionBeforeExecuting pins the ordering: a +// transaction opened after the microflow ran would roll back nothing. +func TestEndpointStartsTheTransactionBeforeExecuting(t *testing.T) { + start := strings.Index(endpointJava, "ctx.startTransaction();") + execute := strings.Index(endpointJava, "Core.microflowCall(mf).execute") + if start < 0 || execute < 0 { + t.Fatal("a landmark is missing") + } + if start > execute { + t.Error("the transaction is started after the microflow runs, so it would roll back nothing") + } +} + +// TestEndpointReportsRollbackOutcome pins that a rollback which fails is +// reported rather than swallowed — otherwise the data stays and the run still +// says PASS. +func TestEndpointReportsRollbackOutcome(t *testing.T) { + for _, want := range []string{`\"rolledBack\":`, `\"rollbackRequested\":`, `\"rollbackError\":`} { + if !strings.Contains(endpointJava, want) { + t.Errorf("the response does not carry %s", want) + } + } +} + +func TestReportRollbackFailureDistinguishesCauses(t *testing.T) { + tc := TestCase{Name: "some test"} + + tests := []struct { + name string + resp runResponse + want string + }{ + { + name: "an endpoint that ignores the parameter", + resp: runResponse{RollbackRequested: false}, + want: "older test endpoint", + }, + { + name: "a runtime that refused", + resp: runResponse{RollbackRequested: true, RollbackError: "transaction already ended"}, + want: "transaction already ended", + }, + { + name: "no reason given", + resp: runResponse{RollbackRequested: true}, + want: "no reason reported", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + reportRollbackFailure(&buf, tc, &tt.resp) + if !strings.Contains(buf.String(), tt.want) { + t.Errorf("warning %q does not mention %q", buf.String(), tt.want) + } + if !strings.Contains(buf.String(), tc.Name) { + t.Errorf("warning %q does not name the test", buf.String()) + } + }) + } +} + +func TestRollbackNote(t *testing.T) { + tests := []struct { + name string + requested bool + resp runResponse + want string + }{ + {"committed", false, runResponse{}, "committed"}, + {"rolled back", true, runResponse{RolledBack: true}, "rolled back"}, + {"failed", true, runResponse{}, "ROLLBACK FAILED"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := rollbackNote(tt.requested, &tt.resp); !strings.Contains(got, tt.want) { + t.Errorf("rollbackNote = %q, want it to contain %q", got, tt.want) + } + }) + } +} + +// TestDescribeStartup pins the line that mxcli-formula1 findings #19 asked for. +// The runner used to say only that after-startup had been repointed, leaving no +// way to tell that the app's own startup logic would not run — which produced a +// suite that passed under --attach and failed under --local for reasons +// unrelated to the code. +func TestDescribeStartup(t *testing.T) { + tests := []struct { + name string + app string + skipped bool + want []string + absent []string + }{ + { + name: "chains the project's own microflow by default", + app: "MyModule.ASU_Startup", + want: []string{"then runs your MyModule.ASU_Startup"}, + // It must not read as though the app's startup is being skipped. + absent: []string{"NOT run"}, + }, + { + name: "says plainly when it is skipped", + app: "MyModule.ASU_Startup", + skipped: true, + want: []string{"MyModule.ASU_Startup", "NOT run", "--skip-app-startup"}, + }, + { + name: "says when there is nothing to chain", + app: "", + want: []string{"no after-startup microflow of its own"}, + absent: []string{"NOT run"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := describeStartup(tt.app, tt.skipped) + for _, w := range tt.want { + if !strings.Contains(got, w) { + t.Errorf("message %q does not contain %q", got, w) + } + } + for _, a := range tt.absent { + if strings.Contains(got, a) { + t.Errorf("message %q should not contain %q", got, a) + } + } + }) + } +} + +// TestSkippedStartupNamesTheFlagThatCausedIt keeps the skipped message +// actionable: a reader who did not pass the flag themselves (a script did) can +// still tell why their startup logic is missing. +func TestSkippedStartupNamesTheFlagThatCausedIt(t *testing.T) { + got := describeStartup("Mod.Flow", true) + if !strings.Contains(got, "--skip-app-startup") { + t.Errorf("message %q does not name the flag responsible", got) + } +} diff --git a/cmd/mxcli/testrunner/client.go b/cmd/mxcli/testrunner/client.go index 27253f415..63880f461 100644 --- a/cmd/mxcli/testrunner/client.go +++ b/cmd/mxcli/testrunner/client.go @@ -47,6 +47,13 @@ type runResponse struct { DurationMicros int64 `json:"durationMicros"` Result string `json:"result"` Error string `json:"error"` + // RollbackRequested echoes back whether the runner asked for a rollback, so a + // runner talking to an older endpoint that ignores the parameter can tell. + RollbackRequested bool `json:"rollbackRequested"` + // RolledBack reports that the transaction was actually rolled back. + RolledBack bool `json:"rolledBack"` + // RollbackError is why it was not. + RollbackError string `json:"rollbackError"` } // listResponse is the endpoint's reply to a list request. @@ -111,10 +118,16 @@ func (c *endpointClient) list() ([]string, error) { return lr.Microflows, nil } -// run executes one test microflow and returns the endpoint's reply. -func (c *endpointClient) run(mf string) (*runResponse, error) { +// run executes one test microflow and returns the endpoint's reply. With +// rollback set, the endpoint wraps the call in a transaction it rolls back, so +// the test's database writes do not survive. +func (c *endpointClient) run(mf string, rollback bool) (*runResponse, error) { + params := url.Values{"mf": {mf}} + if rollback { + params.Set("rollback", "1") + } var rr runResponse - if err := c.get("run", url.Values{"mf": {mf}}, &rr); err != nil { + if err := c.get("run", params, &rr); err != nil { return nil, err } return &rr, nil diff --git a/cmd/mxcli/testrunner/client_test.go b/cmd/mxcli/testrunner/client_test.go index a95b660f1..582ecc544 100644 --- a/cmd/mxcli/testrunner/client_test.go +++ b/cmd/mxcli/testrunner/client_test.go @@ -21,6 +21,8 @@ type fakeEndpoint struct { // seenTokens records what each request presented, so a test can assert the // client actually sends the token rather than the server merely allowing it. seenTokens []string + // rollbackParams records the rollback query parameter of each run request. + rollbackParams []string } func (f *fakeEndpoint) handler() http.Handler { @@ -45,6 +47,7 @@ func (f *fakeEndpoint) handler() http.Handler { } json.NewEncoder(w).Encode(listResponse{Microflows: names}) case strings.HasSuffix(r.URL.Path, "/run"): + f.rollbackParams = append(f.rollbackParams, r.URL.Query().Get("rollback")) mf := r.URL.Query().Get("mf") resp, ok := f.flows[mf] if !ok { @@ -215,3 +218,29 @@ func TestWaitReadyGivesUp(t *testing.T) { t.Errorf("error %q does not explain the endpoint never came up", err) } } + +// TestClientSendsTheRollbackParameter pins that the runner's per-test decision +// actually reaches the endpoint. Without the parameter the endpoint commits, and +// a test annotated for rollback would silently leave its data behind. +func TestClientSendsTheRollbackParameter(t *testing.T) { + fake, c := newFakeEndpoint(t, "tok", map[string]runResponse{ + testFlowPrefix + "test_1": {OK: true, Result: verdictPass}, + }) + + if _, err := c.run(testFlowPrefix+"test_1", true); err != nil { + t.Fatalf("run with rollback: %v", err) + } + if _, err := c.run(testFlowPrefix+"test_1", false); err != nil { + t.Fatalf("run without rollback: %v", err) + } + + if len(fake.rollbackParams) != 2 { + t.Fatalf("server saw %d run requests, want 2", len(fake.rollbackParams)) + } + if fake.rollbackParams[0] != "1" { + t.Errorf("rollback run sent rollback=%q, want \"1\"", fake.rollbackParams[0]) + } + if fake.rollbackParams[1] != "" { + t.Errorf("non-rollback run sent rollback=%q, want it absent", fake.rollbackParams[1]) + } +} diff --git a/cmd/mxcli/testrunner/endpoint.go b/cmd/mxcli/testrunner/endpoint.go index 86a2fea14..3f5cb4e30 100644 --- a/cmd/mxcli/testrunner/endpoint.go +++ b/cmd/mxcli/testrunner/endpoint.go @@ -235,10 +235,23 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex return; } + // rollback=1 wraps the call in a transaction this handler owns and rolls + // it back afterwards, so the test's database writes do not survive it. + // The microflow joins that transaction rather than committing its own — + // Mendix contexts carry one transaction, and a nested start/end only + // adjusts its depth, so the outer rollback undoes everything inside. + boolean rollback = "1".equals(request.getParameter("rollback")); + long t0 = System.nanoTime(); com.mendix.systemwideinterfaces.core.IContext ctx = com.mendix.core.Core.createSystemContext(); Object result = null; String error = null; + String rollbackError = null; + boolean rolledBack = false; + + if (rollback) { + ctx.startTransaction(); + } try { result = com.mendix.core.Core.microflowCall(mf).execute(ctx); } catch (Throwable t) { @@ -246,6 +259,21 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex while (root.getCause() != null && root.getCause() != root) root = root.getCause(); String msg = root.getMessage(); error = (msg == null || msg.isEmpty()) ? root.getClass().getName() : msg; + } finally { + if (rollback) { + // A rollback that silently fails leaves the data behind while the + // run still reports a clean pass, so its outcome is reported + // rather than swallowed. A microflow that already threw may have + // ended the transaction itself; that is not an error worth + // failing the test over, but it is worth saying. + try { + ctx.rollbackTransaction(); + rolledBack = true; + } catch (Throwable t) { + String msg = t.getMessage(); + rollbackError = (msg == null || msg.isEmpty()) ? t.getClass().getName() : msg; + } + } } long micros = (System.nanoTime() - t0) / 1000L; @@ -255,6 +283,9 @@ com.mendix.core.Core.addRequestHandler("` + endpointPath + `", new com.mendix.ex b.append(",\"durationMicros\":").append(micros); b.append(",\"result\":").append(result == null ? "null" : esc(String.valueOf(result))); if (error != null) b.append(",\"error\":").append(esc(error)); + b.append(",\"rollbackRequested\":").append(rollback); + b.append(",\"rolledBack\":").append(rolledBack); + if (rollbackError != null) b.append(",\"rollbackError\":").append(esc(rollbackError)); b.append('}'); out.write(b.toString()); out.flush(); diff --git a/cmd/mxcli/testrunner/parser.go b/cmd/mxcli/testrunner/parser.go index ae276ac81..aff53633b 100644 --- a/cmd/mxcli/testrunner/parser.go +++ b/cmd/mxcli/testrunner/parser.go @@ -134,6 +134,10 @@ func parseMDLTests(content string, sourcePath string) ([]TestCase, error) { continue } + if err := validateCleanup(annotations.Cleanup); err != nil { + return nil, fmt.Errorf("%s: test %q: %w", sourcePath, annotations.Test, err) + } + testID := fmt.Sprintf("test_%d", i+1) tests = append(tests, TestCase{ ID: testID, @@ -185,6 +189,10 @@ func parseMarkdownTests(content string, sourcePath string) ([]TestCase, error) { doc, body, _ := extractDocAndBody(blockContent, blockContent) annotations := parseAnnotations(doc) + if err := validateCleanup(annotations.Cleanup); err != nil { + return nil, fmt.Errorf("%s: test at line %d: %w", sourcePath, blockStart, err) + } + testNum++ testID := fmt.Sprintf("test_%d", testNum) diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index ffc8bf459..33e7e903e 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -58,6 +58,16 @@ type RunOptions struct { // then run against that app's database rather than a scratch one. Attach bool + // SkipAppStartup stops the project's own after-startup microflow from running + // during a --local test run. + // + // It normally does run: the generated startup flow registers the endpoint and + // then chains it, so tests see the app in the state it actually boots into. + // Set this when the suite wants an empty, deterministic baseline instead — + // e.g. the app seeds demo data at startup and the tests are asserting on + // counts. + SkipAppStartup bool + // Timeout for runtime startup and test execution. Timeout time.Duration @@ -178,10 +188,21 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. return nil, err } + // Capture what cleanup will need to restore, before touching anything. This + // must succeed: without it cleanup cannot tell an existing MxTest module from + // the one it is about to create, nor restore the original after-startup — and + // the generated startup flow needs to know what to chain. + state, err := captureProjectState(opts.ProjectPath) + if err != nil { + return nil, fmt.Errorf("capturing project state: %w", err) + } + fmt.Fprintln(w, "Generating test endpoint and test microflows...") - // "" : a test run wants a known starting state, so the project's own - // after-startup is not chained here (a hosted endpoint does chain it). - endpointMDL := GenerateEndpointMDL("") + chain := state.afterStartup + if opts.SkipAppStartup { + chain = "" + } + endpointMDL := GenerateEndpointMDL(chain) flowsMDL := GenerateTestFlows(suite) if opts.Verbose { @@ -191,14 +212,7 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. fmt.Fprintln(w, "--- End MDL ---") } - // Capture what cleanup will need to restore, before touching anything. This - // must succeed: without it cleanup cannot tell an existing MxTest module from - // the one it is about to create, nor restore the original after-startup. fmt.Fprintln(w, "Injecting test endpoint into project...") - state, err := captureProjectState(opts.ProjectPath) - if err != nil { - return nil, fmt.Errorf("capturing project state: %w", err) - } // From here on the project is modified, so every exit runs cleanup. // @@ -235,7 +249,7 @@ func runEndpoint(opts RunOptions, suite *TestSuite, timeout time.Duration, w io. return finish(nil, fmt.Errorf("preparing project for the test run (%s): %w", cmd, err)) } } - fmt.Fprintf(w, " After-startup set to %s (registers the endpoint; runs no tests)\n", endpointStartupFlow) + fmt.Fprintln(w, " "+describeStartup(state.afterStartup, opts.SkipAppStartup)) // --watch keeps the runtime and the build server up and re-runs on every // change, so it owns the loop — including printing each run's results, which diff --git a/cmd/mxcli/testrunner/runner_endpoint.go b/cmd/mxcli/testrunner/runner_endpoint.go index c3d67c0de..969c33d56 100644 --- a/cmd/mxcli/testrunner/runner_endpoint.go +++ b/cmd/mxcli/testrunner/runner_endpoint.go @@ -129,6 +129,9 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr result := &SuiteResult{Name: suite.Name, Started: time.Now()} fmt.Fprintf(w, "Running %d test(s) over the test endpoint...\n", len(suite.Tests)) + // leaked counts tests whose requested rollback did not happen. + leaked := 0 + for _, tc := range suite.Tests { flow := testFlowName(tc) if !present[flow] { @@ -141,7 +144,8 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr continue } - rr, err := client.run(flow) + rollback := rollsBack(tc) + rr, err := client.run(flow, rollback) if err != nil { // A transport failure is not a verdict. Report it against this test // and keep going; if the runtime died the rest will say so too. @@ -154,13 +158,27 @@ func runSuite(client *endpointClient, suite *TestSuite, opts RunOptions, w io.Wr continue } + // A rollback that was asked for and did not happen leaves the test's data + // in the database while the verdict still says PASS. The test itself is + // not wrong, so its verdict stands — but this must not pass in silence. + if rollback && !rr.RolledBack { + leaked++ + reportRollbackFailure(w, tc, rr) + } + res := toResult(tc, rr) result.Tests = append(result.Tests, res) if opts.Verbose { - fmt.Fprintf(w, " %s %s (%s)\n", res.Status, res.Name, res.Duration.Round(time.Millisecond)) + fmt.Fprintf(w, " %s %s (%s)%s\n", res.Status, res.Name, + res.Duration.Round(time.Millisecond), rollbackNote(rollback, rr)) } } + if leaked > 0 { + fmt.Fprintf(w, "\nWARNING: %d test(s) asked for @cleanup rollback and did not get it — "+ + "their writes are still in the database.\n", leaked) + } + result.Duration = time.Since(result.Started) return result, nil } diff --git a/cmd/mxcli/theme/assets/console/files/theme/web/_mxcli-widgets.scss b/cmd/mxcli/theme/assets/console/files/theme/web/_mxcli-widgets.scss new file mode 100644 index 000000000..3e7073865 --- /dev/null +++ b/cmd/mxcli/theme/assets/console/files/theme/web/_mxcli-widgets.scss @@ -0,0 +1,167 @@ +// The widget-module layer — shared by every mxcli theme, identical in each. +// +// _mxcli-atlas-map.scss re-points Atlas Core's CSS custom properties at the +// palette, and that covers the app: ground, surfaces, ink, brand, type, cards, +// buttons and form controls all follow. What it cannot reach is the theme +// source shipped by the *widget modules* under themesource/, which styles a +// number of things with Sass variables and literals. Sass resolves those at +// compile time, before any custom property exists, so the value is baked into +// theme.compiled.css and no --mxt-* can move it. Only a later CSS rule can. +// +// This file is that rule set. It is imported after the theme partial, so it +// wins on source order without !important, and every declaration resolves +// through a token so both palettes follow. +// +// THE OBVIOUS FIX DOES NOT WORK. Each module's main.scss imports +// theme/web/custom-variables *before* its own `!default` variables, so setting +// e.g. `$pagination-caption-color: var(--mxt-ink-muted)` there would win, and +// Sass would substitute the var() reference into every use site. It is a real +// technique — but not here, for two reasons: +// +// 1. The names collide with Atlas Core's own, and Atlas Core feeds them to +// Sass colour functions: atlas_core/web/_variables.scss:20 computes +// `mix($brand-primary, #e7e7e9, 10%)`. Handing mix() a var() reference is +// a compile error, so the app stops building. +// 2. The worst offenders are not behind a variable at all. +// _three-state-checkbox.scss writes #264ae5 and rgba(#264ae5, 0.4) +// directly, so overriding $brand-primary would not reach them anyway. +// +// Every selector below was read out of a compiled theme.compiled.css, not from +// the SCSS sources and not guessed — the sources contain many +// `var(--token, #fallback)` declarations that already resolve correctly, and +// only the bare literals are actually a problem. Each rule names the source it +// corrects. + +// --------------------------------------------------------------------------- +// Data Grid 2 (themesource/datawidgets) +// --------------------------------------------------------------------------- + +// variables.scss:18 — $pagination-caption-color: #0a1325, a Sass variable, so +// unreachable. This is the caption that reads "1–15 of 77": the only thing +// telling a user where they are in the result set. Against a dark ground it +// measured 1.02:1 and was simply invisible. The pager *buttons* either side +// were always fine, because they resolve var(--gray-darker, …) through Atlas — +// same bar, two mechanisms, one of them reachable. +.pagination-bar { + color: var(--mxt-ink-muted); +} + +// _datagrid.scss:442 — background-color: rgba(255, 255, 255, 1). A full-width +// panel that replaces the rows while a page loads, so every page turn flashed +// white on a dark app. +.widget-datagrid-loader-container { + background-color: var(--mxt-surface); +} + +// _datagrid.scss:212-214 and 370-372 — box-shadow: 0 2px 20px 1px +// rgba(32, 43, 54, 0.08). A light-mode drop shadow under the column selector, +// which on a dark ground reads as a smudge rather than elevation. Themes that +// set --mxt-shadow: none get a hairline instead, which is what carries the +// separation for them. +.table .column-selector .column-selector-content .column-selectors, +.column-selectors { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _datagrid-filters.scss:71, 143, 153 and 206 — the same two-layer light-mode +// shadow, baked four times: +// +// box-shadow: 0 2px 20px 1px rgba(5, 15, 129, .05), +// 0 2px 16px 0 rgba(33, 43, 54, .08); +// +// These are the filter-operator popover, the dropdown filter's list in both its +// standalone and contained forms, and the list inside a dropdown container. Each +// already takes its background from --bg-color-secondary, so Atlas re-colours the +// panel and leaves the shadow behind — elevation drawn for a light ground, +// floating over a dark one. Same treatment as .column-selectors above. +.filter-selectors, +.dropdown-content, +:not(.dropdown-content) > .dropdown-list, +.dropdown-container .dropdown-list { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _three-state-checkbox.scss — the row-select boxes down the left of every +// grid. Nine baked literals: #e7e7e9 borders and #f8f8f8 disabled fills that +// vanish on a dark surface, #ffffff checkmarks, and #264ae5 for the checked and +// indeterminate states — the one place stock Mendix blue still showed through a +// re-branded app. +input[type="checkbox"].three-state-checkbox { + &:before { + border-color: var(--mxt-line); + } + + &:not(:indeterminate):after, + &:indeterminate:after { + border-color: var(--mxt-brand-ink); + } + + &:not(:disabled):not(:checked):hover:after { + border-color: var(--mxt-line); + } + + &:indeterminate:before, + &:checked:before { + border-color: var(--mxt-brand); + background-color: var(--mxt-brand); + } + + &:disabled:before, + &:disabled:after { + border-color: var(--mxt-surface-alt); + background-color: var(--mxt-surface-alt); + } + + &:indeterminate:disabled:before, + &:checked:disabled:before { + border-color: transparent; + background-color: var(--mxt-surface-alt); + } +} + +// _datagrid-dropdown-filter.scss:246 — --wdf-input-placeholder-color: +// rgb(117, 117, 117), a mid grey chosen for a white field. +:where(.widget-dropdown-filter.variant-select[data-empty]) { + --wdf-input-placeholder-color: var(--mxt-ink-faint); +} + +// _datagrid-dropdown-filter.scss:267 — color: #000 on the selected tag, black +// text on a chip that is no longer light. +:where(.widget-dropdown-filter.variant-tag-picker) .widget-dropdown-filter-selected-item, +:where(.widget-dropdown-filter.variant-tag-picker-text) .widget-dropdown-filter-selected-item { + color: var(--mxt-ink); +} + +// _export-alert.scss — the cancel button's focus ring, the one part of the +// export UI that is a bare literal rather than a var() fallback. +.widget-datagrid-export-alert-cancel.btn:focus-visible { + outline-color: var(--mxt-brand); +} + +// --------------------------------------------------------------------------- +// Other modules that bake the stock brand colour +// +// These render only if the module is present — the feedback widget ships in a +// blank app and draws a floating button over every page. Harmless when absent: +// an unmatched selector costs nothing. +// --------------------------------------------------------------------------- +.mxfeedback-start-button--side { + background-color: var(--mxt-brand); +} + +.mxfeedback-tool-button--active, +.mxfeedback-screenshot-preview__delete-button { + color: var(--mxt-brand); +} + +.mxfeedback-tool-button__color:focus, +.mxfeedback-tool-button__thickness:focus { + outline-color: var(--mxt-brand); +} + +.take-picture-save-button { + background-color: var(--mxt-brand); + color: var(--mxt-brand-ink); +} diff --git a/cmd/mxcli/theme/assets/console/files/theme/web/main.scss b/cmd/mxcli/theme/assets/console/files/theme/web/main.scss index fa97920ad..ee5c07a0a 100644 --- a/cmd/mxcli/theme/assets/console/files/theme/web/main.scss +++ b/cmd/mxcli/theme/assets/console/files/theme/web/main.scss @@ -7,3 +7,4 @@ $mxcli-theme-variant: {{VARIANT}}; @import "mxcli-atlas-map"; @import "mxcli-console"; +@import "mxcli-widgets"; diff --git a/cmd/mxcli/theme/assets/console/theme.json b/cmd/mxcli/theme/assets/console/theme.json index 1d151bde0..ce9cbc3af 100644 --- a/cmd/mxcli/theme/assets/console/theme.json +++ b/cmd/mxcli/theme/assets/console/theme.json @@ -29,6 +29,11 @@ "mode": "block", "purpose": "Layer 2 \u2014 light palette, variant blocks, fonts, recipes" }, + { + "path": "theme/web/_mxcli-widgets.scss", + "mode": "block", + "purpose": "the widget-module layer \u2014 rules for colours Sass bakes before any token exists" + }, { "path": "theme/web/main.scss", "mode": "block", diff --git a/cmd/mxcli/theme/assets/ledger/files/theme/web/_mxcli-widgets.scss b/cmd/mxcli/theme/assets/ledger/files/theme/web/_mxcli-widgets.scss new file mode 100644 index 000000000..3e7073865 --- /dev/null +++ b/cmd/mxcli/theme/assets/ledger/files/theme/web/_mxcli-widgets.scss @@ -0,0 +1,167 @@ +// The widget-module layer — shared by every mxcli theme, identical in each. +// +// _mxcli-atlas-map.scss re-points Atlas Core's CSS custom properties at the +// palette, and that covers the app: ground, surfaces, ink, brand, type, cards, +// buttons and form controls all follow. What it cannot reach is the theme +// source shipped by the *widget modules* under themesource/, which styles a +// number of things with Sass variables and literals. Sass resolves those at +// compile time, before any custom property exists, so the value is baked into +// theme.compiled.css and no --mxt-* can move it. Only a later CSS rule can. +// +// This file is that rule set. It is imported after the theme partial, so it +// wins on source order without !important, and every declaration resolves +// through a token so both palettes follow. +// +// THE OBVIOUS FIX DOES NOT WORK. Each module's main.scss imports +// theme/web/custom-variables *before* its own `!default` variables, so setting +// e.g. `$pagination-caption-color: var(--mxt-ink-muted)` there would win, and +// Sass would substitute the var() reference into every use site. It is a real +// technique — but not here, for two reasons: +// +// 1. The names collide with Atlas Core's own, and Atlas Core feeds them to +// Sass colour functions: atlas_core/web/_variables.scss:20 computes +// `mix($brand-primary, #e7e7e9, 10%)`. Handing mix() a var() reference is +// a compile error, so the app stops building. +// 2. The worst offenders are not behind a variable at all. +// _three-state-checkbox.scss writes #264ae5 and rgba(#264ae5, 0.4) +// directly, so overriding $brand-primary would not reach them anyway. +// +// Every selector below was read out of a compiled theme.compiled.css, not from +// the SCSS sources and not guessed — the sources contain many +// `var(--token, #fallback)` declarations that already resolve correctly, and +// only the bare literals are actually a problem. Each rule names the source it +// corrects. + +// --------------------------------------------------------------------------- +// Data Grid 2 (themesource/datawidgets) +// --------------------------------------------------------------------------- + +// variables.scss:18 — $pagination-caption-color: #0a1325, a Sass variable, so +// unreachable. This is the caption that reads "1–15 of 77": the only thing +// telling a user where they are in the result set. Against a dark ground it +// measured 1.02:1 and was simply invisible. The pager *buttons* either side +// were always fine, because they resolve var(--gray-darker, …) through Atlas — +// same bar, two mechanisms, one of them reachable. +.pagination-bar { + color: var(--mxt-ink-muted); +} + +// _datagrid.scss:442 — background-color: rgba(255, 255, 255, 1). A full-width +// panel that replaces the rows while a page loads, so every page turn flashed +// white on a dark app. +.widget-datagrid-loader-container { + background-color: var(--mxt-surface); +} + +// _datagrid.scss:212-214 and 370-372 — box-shadow: 0 2px 20px 1px +// rgba(32, 43, 54, 0.08). A light-mode drop shadow under the column selector, +// which on a dark ground reads as a smudge rather than elevation. Themes that +// set --mxt-shadow: none get a hairline instead, which is what carries the +// separation for them. +.table .column-selector .column-selector-content .column-selectors, +.column-selectors { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _datagrid-filters.scss:71, 143, 153 and 206 — the same two-layer light-mode +// shadow, baked four times: +// +// box-shadow: 0 2px 20px 1px rgba(5, 15, 129, .05), +// 0 2px 16px 0 rgba(33, 43, 54, .08); +// +// These are the filter-operator popover, the dropdown filter's list in both its +// standalone and contained forms, and the list inside a dropdown container. Each +// already takes its background from --bg-color-secondary, so Atlas re-colours the +// panel and leaves the shadow behind — elevation drawn for a light ground, +// floating over a dark one. Same treatment as .column-selectors above. +.filter-selectors, +.dropdown-content, +:not(.dropdown-content) > .dropdown-list, +.dropdown-container .dropdown-list { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _three-state-checkbox.scss — the row-select boxes down the left of every +// grid. Nine baked literals: #e7e7e9 borders and #f8f8f8 disabled fills that +// vanish on a dark surface, #ffffff checkmarks, and #264ae5 for the checked and +// indeterminate states — the one place stock Mendix blue still showed through a +// re-branded app. +input[type="checkbox"].three-state-checkbox { + &:before { + border-color: var(--mxt-line); + } + + &:not(:indeterminate):after, + &:indeterminate:after { + border-color: var(--mxt-brand-ink); + } + + &:not(:disabled):not(:checked):hover:after { + border-color: var(--mxt-line); + } + + &:indeterminate:before, + &:checked:before { + border-color: var(--mxt-brand); + background-color: var(--mxt-brand); + } + + &:disabled:before, + &:disabled:after { + border-color: var(--mxt-surface-alt); + background-color: var(--mxt-surface-alt); + } + + &:indeterminate:disabled:before, + &:checked:disabled:before { + border-color: transparent; + background-color: var(--mxt-surface-alt); + } +} + +// _datagrid-dropdown-filter.scss:246 — --wdf-input-placeholder-color: +// rgb(117, 117, 117), a mid grey chosen for a white field. +:where(.widget-dropdown-filter.variant-select[data-empty]) { + --wdf-input-placeholder-color: var(--mxt-ink-faint); +} + +// _datagrid-dropdown-filter.scss:267 — color: #000 on the selected tag, black +// text on a chip that is no longer light. +:where(.widget-dropdown-filter.variant-tag-picker) .widget-dropdown-filter-selected-item, +:where(.widget-dropdown-filter.variant-tag-picker-text) .widget-dropdown-filter-selected-item { + color: var(--mxt-ink); +} + +// _export-alert.scss — the cancel button's focus ring, the one part of the +// export UI that is a bare literal rather than a var() fallback. +.widget-datagrid-export-alert-cancel.btn:focus-visible { + outline-color: var(--mxt-brand); +} + +// --------------------------------------------------------------------------- +// Other modules that bake the stock brand colour +// +// These render only if the module is present — the feedback widget ships in a +// blank app and draws a floating button over every page. Harmless when absent: +// an unmatched selector costs nothing. +// --------------------------------------------------------------------------- +.mxfeedback-start-button--side { + background-color: var(--mxt-brand); +} + +.mxfeedback-tool-button--active, +.mxfeedback-screenshot-preview__delete-button { + color: var(--mxt-brand); +} + +.mxfeedback-tool-button__color:focus, +.mxfeedback-tool-button__thickness:focus { + outline-color: var(--mxt-brand); +} + +.take-picture-save-button { + background-color: var(--mxt-brand); + color: var(--mxt-brand-ink); +} diff --git a/cmd/mxcli/theme/assets/ledger/files/theme/web/main.scss b/cmd/mxcli/theme/assets/ledger/files/theme/web/main.scss index 260016738..0ae379c9e 100644 --- a/cmd/mxcli/theme/assets/ledger/files/theme/web/main.scss +++ b/cmd/mxcli/theme/assets/ledger/files/theme/web/main.scss @@ -7,3 +7,4 @@ $mxcli-theme-variant: {{VARIANT}}; @import "mxcli-atlas-map"; @import "mxcli-ledger"; +@import "mxcli-widgets"; diff --git a/cmd/mxcli/theme/assets/ledger/theme.json b/cmd/mxcli/theme/assets/ledger/theme.json index 988837b18..b9c06c4ee 100644 --- a/cmd/mxcli/theme/assets/ledger/theme.json +++ b/cmd/mxcli/theme/assets/ledger/theme.json @@ -29,6 +29,11 @@ "mode": "block", "purpose": "Layer 2 \u2014 dark palette, variant blocks, fonts, recipes" }, + { + "path": "theme/web/_mxcli-widgets.scss", + "mode": "block", + "purpose": "the widget-module layer \u2014 rules for colours Sass bakes before any token exists" + }, { "path": "theme/web/main.scss", "mode": "block", diff --git a/cmd/mxcli/theme/assets/signal/files/theme/web/_mxcli-widgets.scss b/cmd/mxcli/theme/assets/signal/files/theme/web/_mxcli-widgets.scss new file mode 100644 index 000000000..3e7073865 --- /dev/null +++ b/cmd/mxcli/theme/assets/signal/files/theme/web/_mxcli-widgets.scss @@ -0,0 +1,167 @@ +// The widget-module layer — shared by every mxcli theme, identical in each. +// +// _mxcli-atlas-map.scss re-points Atlas Core's CSS custom properties at the +// palette, and that covers the app: ground, surfaces, ink, brand, type, cards, +// buttons and form controls all follow. What it cannot reach is the theme +// source shipped by the *widget modules* under themesource/, which styles a +// number of things with Sass variables and literals. Sass resolves those at +// compile time, before any custom property exists, so the value is baked into +// theme.compiled.css and no --mxt-* can move it. Only a later CSS rule can. +// +// This file is that rule set. It is imported after the theme partial, so it +// wins on source order without !important, and every declaration resolves +// through a token so both palettes follow. +// +// THE OBVIOUS FIX DOES NOT WORK. Each module's main.scss imports +// theme/web/custom-variables *before* its own `!default` variables, so setting +// e.g. `$pagination-caption-color: var(--mxt-ink-muted)` there would win, and +// Sass would substitute the var() reference into every use site. It is a real +// technique — but not here, for two reasons: +// +// 1. The names collide with Atlas Core's own, and Atlas Core feeds them to +// Sass colour functions: atlas_core/web/_variables.scss:20 computes +// `mix($brand-primary, #e7e7e9, 10%)`. Handing mix() a var() reference is +// a compile error, so the app stops building. +// 2. The worst offenders are not behind a variable at all. +// _three-state-checkbox.scss writes #264ae5 and rgba(#264ae5, 0.4) +// directly, so overriding $brand-primary would not reach them anyway. +// +// Every selector below was read out of a compiled theme.compiled.css, not from +// the SCSS sources and not guessed — the sources contain many +// `var(--token, #fallback)` declarations that already resolve correctly, and +// only the bare literals are actually a problem. Each rule names the source it +// corrects. + +// --------------------------------------------------------------------------- +// Data Grid 2 (themesource/datawidgets) +// --------------------------------------------------------------------------- + +// variables.scss:18 — $pagination-caption-color: #0a1325, a Sass variable, so +// unreachable. This is the caption that reads "1–15 of 77": the only thing +// telling a user where they are in the result set. Against a dark ground it +// measured 1.02:1 and was simply invisible. The pager *buttons* either side +// were always fine, because they resolve var(--gray-darker, …) through Atlas — +// same bar, two mechanisms, one of them reachable. +.pagination-bar { + color: var(--mxt-ink-muted); +} + +// _datagrid.scss:442 — background-color: rgba(255, 255, 255, 1). A full-width +// panel that replaces the rows while a page loads, so every page turn flashed +// white on a dark app. +.widget-datagrid-loader-container { + background-color: var(--mxt-surface); +} + +// _datagrid.scss:212-214 and 370-372 — box-shadow: 0 2px 20px 1px +// rgba(32, 43, 54, 0.08). A light-mode drop shadow under the column selector, +// which on a dark ground reads as a smudge rather than elevation. Themes that +// set --mxt-shadow: none get a hairline instead, which is what carries the +// separation for them. +.table .column-selector .column-selector-content .column-selectors, +.column-selectors { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _datagrid-filters.scss:71, 143, 153 and 206 — the same two-layer light-mode +// shadow, baked four times: +// +// box-shadow: 0 2px 20px 1px rgba(5, 15, 129, .05), +// 0 2px 16px 0 rgba(33, 43, 54, .08); +// +// These are the filter-operator popover, the dropdown filter's list in both its +// standalone and contained forms, and the list inside a dropdown container. Each +// already takes its background from --bg-color-secondary, so Atlas re-colours the +// panel and leaves the shadow behind — elevation drawn for a light ground, +// floating over a dark one. Same treatment as .column-selectors above. +.filter-selectors, +.dropdown-content, +:not(.dropdown-content) > .dropdown-list, +.dropdown-container .dropdown-list { + border-color: var(--mxt-line); + box-shadow: var(--mxt-shadow); +} + +// _three-state-checkbox.scss — the row-select boxes down the left of every +// grid. Nine baked literals: #e7e7e9 borders and #f8f8f8 disabled fills that +// vanish on a dark surface, #ffffff checkmarks, and #264ae5 for the checked and +// indeterminate states — the one place stock Mendix blue still showed through a +// re-branded app. +input[type="checkbox"].three-state-checkbox { + &:before { + border-color: var(--mxt-line); + } + + &:not(:indeterminate):after, + &:indeterminate:after { + border-color: var(--mxt-brand-ink); + } + + &:not(:disabled):not(:checked):hover:after { + border-color: var(--mxt-line); + } + + &:indeterminate:before, + &:checked:before { + border-color: var(--mxt-brand); + background-color: var(--mxt-brand); + } + + &:disabled:before, + &:disabled:after { + border-color: var(--mxt-surface-alt); + background-color: var(--mxt-surface-alt); + } + + &:indeterminate:disabled:before, + &:checked:disabled:before { + border-color: transparent; + background-color: var(--mxt-surface-alt); + } +} + +// _datagrid-dropdown-filter.scss:246 — --wdf-input-placeholder-color: +// rgb(117, 117, 117), a mid grey chosen for a white field. +:where(.widget-dropdown-filter.variant-select[data-empty]) { + --wdf-input-placeholder-color: var(--mxt-ink-faint); +} + +// _datagrid-dropdown-filter.scss:267 — color: #000 on the selected tag, black +// text on a chip that is no longer light. +:where(.widget-dropdown-filter.variant-tag-picker) .widget-dropdown-filter-selected-item, +:where(.widget-dropdown-filter.variant-tag-picker-text) .widget-dropdown-filter-selected-item { + color: var(--mxt-ink); +} + +// _export-alert.scss — the cancel button's focus ring, the one part of the +// export UI that is a bare literal rather than a var() fallback. +.widget-datagrid-export-alert-cancel.btn:focus-visible { + outline-color: var(--mxt-brand); +} + +// --------------------------------------------------------------------------- +// Other modules that bake the stock brand colour +// +// These render only if the module is present — the feedback widget ships in a +// blank app and draws a floating button over every page. Harmless when absent: +// an unmatched selector costs nothing. +// --------------------------------------------------------------------------- +.mxfeedback-start-button--side { + background-color: var(--mxt-brand); +} + +.mxfeedback-tool-button--active, +.mxfeedback-screenshot-preview__delete-button { + color: var(--mxt-brand); +} + +.mxfeedback-tool-button__color:focus, +.mxfeedback-tool-button__thickness:focus { + outline-color: var(--mxt-brand); +} + +.take-picture-save-button { + background-color: var(--mxt-brand); + color: var(--mxt-brand-ink); +} diff --git a/cmd/mxcli/theme/assets/signal/files/theme/web/main.scss b/cmd/mxcli/theme/assets/signal/files/theme/web/main.scss index 6ebdf074d..48c3e338b 100644 --- a/cmd/mxcli/theme/assets/signal/files/theme/web/main.scss +++ b/cmd/mxcli/theme/assets/signal/files/theme/web/main.scss @@ -7,3 +7,4 @@ $mxcli-theme-variant: {{VARIANT}}; @import "mxcli-atlas-map"; @import "mxcli-signal"; +@import "mxcli-widgets"; diff --git a/cmd/mxcli/theme/assets/signal/theme.json b/cmd/mxcli/theme/assets/signal/theme.json index 9ddc93bf8..c4d35e3a6 100644 --- a/cmd/mxcli/theme/assets/signal/theme.json +++ b/cmd/mxcli/theme/assets/signal/theme.json @@ -28,6 +28,11 @@ "mode": "block", "purpose": "Layer 2 \u2014 dark palette, variant blocks, fonts, recipes" }, + { + "path": "theme/web/_mxcli-widgets.scss", + "mode": "block", + "purpose": "the widget-module layer \u2014 rules for colours Sass bakes before any token exists" + }, { "path": "theme/web/main.scss", "mode": "block", diff --git a/cmd/mxcli/theme/theme_test.go b/cmd/mxcli/theme/theme_test.go index 6af792e72..546624119 100644 --- a/cmd/mxcli/theme/theme_test.go +++ b/cmd/mxcli/theme/theme_test.go @@ -6,6 +6,7 @@ import ( "errors" "os" "path/filepath" + "regexp" "strings" "testing" ) @@ -300,24 +301,56 @@ func TestAllThemesAreWellFormed(t *testing.T) { // The Atlas wiring is what makes a palette swap cheap, so every theme has to // run through the same one. Shipped per theme (a theme package is meant to be // self-contained), which is exactly why it can drift. -func TestAtlasMapIsIdenticalInEveryTheme(t *testing.T) { +func TestSharedPartialsAreIdenticalInEveryTheme(t *testing.T) { themes, err := List() if err != nil { t.Fatal(err) } - var reference []byte - var referenceName string + for _, shared := range []string{"_mxcli-atlas-map.scss", "_mxcli-widgets.scss"} { + var reference []byte + var referenceName string + for _, th := range themes { + body, err := assetsFS.ReadFile("assets/" + th.Name + "/files/theme/web/" + shared) + if err != nil { + t.Fatalf("%s ships no %s: %v", th.Name, shared, err) + } + if reference == nil { + reference, referenceName = body, th.Name + continue + } + if string(body) != string(reference) { + t.Errorf("%s's %s has drifted from %s's", th.Name, shared, referenceName) + } + } + } +} + +// The widget layer exists because Sass bakes these colours before any custom +// property exists, so a rule that reintroduces a literal defeats the point. +func TestWidgetLayerResolvesEveryColourThroughAToken(t *testing.T) { + themes, err := List() + if err != nil { + t.Fatal(err) + } + literal := regexp.MustCompile(`(color|background|background-color|border-color|outline-color|box-shadow)\s*:\s*[^;]*(#[0-9a-fA-F]{3,8}|\brgba?\()`) for _, th := range themes { - body, err := assetsFS.ReadFile("assets/" + th.Name + "/files/theme/web/_mxcli-atlas-map.scss") + body, err := assetsFS.ReadFile("assets/" + th.Name + "/files/theme/web/_mxcli-widgets.scss") if err != nil { - t.Fatalf("%s ships no Atlas map: %v", th.Name, err) + t.Fatal(err) } - if reference == nil { - reference, referenceName = body, th.Name - continue + for i, line := range strings.Split(string(body), "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "//") { + continue + } + if literal.MatchString(trimmed) { + t.Errorf("%s _mxcli-widgets.scss:%d reintroduces a literal colour: %q", + th.Name, i+1, trimmed) + } } - if string(body) != string(reference) { - t.Errorf("%s's Atlas map has drifted from %s's", th.Name, referenceName) + // And it has to actually carry the fix that prompted the layer. + if !strings.Contains(string(body), ".pagination-bar") { + t.Errorf("%s: no rule for the Data Grid 2 pager caption", th.Name) } } } diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 4d45072d2..e79a9d837 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -87,6 +87,54 @@ every request must present that token, non-loopback callers are refused, and it will only ever invoke the generated `MxTest.Test_*` microflows. The token is never written into your project. +### The app's own after-startup microflow + +Boot registers the endpoint and then runs the project's own after-startup +microflow, so tests see the app in the state it really boots into. The run +prints which of the two happened, and `--skip-app-startup` opts out when a suite +wants an empty, deterministic baseline. + +This keeps a suite behaving the same under `--local` and `--attach`. Note that +what the startup microflow writes is not covered by `@cleanup rollback` — it +runs at boot, outside any test's transaction. + +### `@cleanup`: what happens to a test's data + +`rollback` is the **default**, so a test's database writes do not survive it. +The endpoint opens a transaction around the call and rolls it back afterwards, +including when the test throws. + +```mdl +/** + * @test creating an order does not leak + * @expect $result = 'ok' + */ +$result = CALL MICROFLOW Sales.CreateOrder(Amount = 100); +/ + +/** + * @test seed a fixture the app should keep + * @cleanup none + */ +$result = CALL MICROFLOW Sales.SeedCatalogue(); +/ +``` + +| Strategy | Effect | +|---|---| +| `rollback` (default) | The test's writes are rolled back, even if it throws | +| `none` | The writes commit and persist | + +Rollback needs the test endpoint, so it applies to `--local` and `--attach`. +The Docker / `--legacy-runner` path runs tests inside the after-startup action +and has no context of its own to roll back, so it always commits. + +A rollback that fails is reported per test and summarised at the end — data +left behind while the suite still says PASS is exactly what this is for. +`--verbose` tags every test `[rolled back]`, `[committed]` or +`[ROLLBACK FAILED]`. A misspelled strategy is a parse error, not a silent +commit. + **Docker — the after-startup runner.** The whole suite is compiled into the project's after-startup microflow, the container is restarted, and results are parsed out of its log. `--legacy-runner` selects this on a local run too. diff --git a/docs-site/src/tools/theme.md b/docs-site/src/tools/theme.md index cfe4bc97f..bdd491dfb 100644 --- a/docs-site/src/tools/theme.md +++ b/docs-site/src/tools/theme.md @@ -33,14 +33,15 @@ because two themes mapping the same Atlas variables would fight in the cascade. ## What it writes -Five things, all under `theme/`: +Six things, all under `theme/`: | File | What | |---|---| | `theme/web/custom-variables.scss` | the theme's palette — this is the file to edit | | `theme/web/_mxcli-atlas-map.scss` | the Atlas wiring: ~60 Atlas variables expressed in terms of the palette | | `theme/web/_mxcli-.scss` | the other palette, the variant blocks, `@font-face`, recipe classes | -| `theme/web/main.scss` | the variant switch plus two `@import` lines | +| `theme/web/_mxcli-widgets.scss` | the widget-module layer: colours Sass bakes before any token exists | +| `theme/web/main.scss` | the variant switch plus the `@import` lines | | `theme/web/mxcli-fonts/` | vendored fonts (SIL OFL 1.1) | **The model is never touched.** No `.mpr` changes, so nothing here can affect a @@ -137,6 +138,11 @@ mxcli block — anything outside the fence is never touched. - **`themesource//` is only compiled when `` matches a real module**, so a theme never writes there. `theme/web/main.scss` compiles last and is the correct home for app-level styling. +- **The widget modules bake some colours as Sass literals**, before any custom + property exists, so no token can move them — the Data Grid 2 pager caption is + the worst case, at 1.02:1 on a dark ground. `_mxcli-widgets.scss` corrects + those with ordinary rules; it is regenerated with the theme, so leave it alone + and put your own overrides outside the fence. ## Recipe classes diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index ac4e35c32..7775d3167 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -333,6 +333,7 @@ it is for pages. | Statement | Syntax | Notes | |-----------|--------|-------| +| List folders | `list folders [in module];` | The folder layout, with the documents in each folder | | Microflow folder | `folder 'path'` (before BEGIN) | `create microflow ... folder 'ACT' begin ... end;` | | Page folder | `folder: 'path'` (in properties) | `create page ... (folder: 'pages/Detail') { ... }` | | Drop folder | `drop folder 'path' in module;` | Folder must be empty | @@ -355,7 +356,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Show demo users | `show demo users;` | Configured demo users | | Show access on element | `show access on microflow\|nanoflow\|page\|entity Mod.Name;` | Which roles can access | | Show security matrix | `show security matrix [in module];` | Full access overview | -| Create module role | `create module role Mod.Role [description 'text'];` | | +| Create module role | `create [or modify] module role Mod.Role [description 'text'];` | `or modify` updates an existing role instead of failing, so a security script can be re-run | | Drop module role | `drop module role Mod.Role;` | | | Create user role | `create user role Name (Mod.Role, ...) [manage all roles];` | Aggregates module roles | | Alter user role | `alter user role Name add\|remove module roles (Mod.Role, ...);` | | @@ -485,6 +486,7 @@ alter workflow Module.OrderApproval | Full types | `show structure depth 3;` | Typed attributes, named parameters | | Filter by module | `show structure in ModuleName;` | Single module only | | Include all modules | `show structure depth 1 all;` | Include system/marketplace modules | +| Folder layout | `list folders [in module];` | `show structure` is by document type at every depth and never shows folders — use this to read back where a `move` put something | ## Navigation diff --git a/mdl-examples/bug-tests/365-microflow-inheritance-split.mdl b/mdl-examples/bug-tests/365-microflow-inheritance-split.mdl index b938d49c0..879a21753 100644 --- a/mdl-examples/bug-tests/365-microflow-inheritance-split.mdl +++ b/mdl-examples/bug-tests/365-microflow-inheritance-split.mdl @@ -10,17 +10,29 @@ create persistent entity InheritanceSplitExample.SpecializedInput extends Inheri ); / +-- An object-type decision needs an outgoing flow for EVERY listed type, +-- including the base entity. This example used `case Specialized` + `else`, +-- which fails the build with CE0090 ("The 'InheritanceSplitExample.BaseInput' +-- value should be configured for an outgoing flow") — `else` serializes as +-- Microflows$NoCase and is accepted, but it does not satisfy coverage. +-- +-- The branches also converge on a merge that continues to the end event, so a +-- non-void microflow needs a `return` after `end split;` (otherwise CE0067 +-- "The 'Return value' property is required", and mxcli check reports MDL003). +-- +-- Verified with mxbuild 11.6.6 and 11.13.0: 0 errors. create microflow InheritanceSplitExample.RouteInput ( $Input: InheritanceSplitExample.BaseInput ) returns boolean begin + declare $IsSpecialized boolean = false; split type $Input case InheritanceSplitExample.SpecializedInput cast $SpecializedInput; - return true; - else - return false; + set $IsSpecialized = true; + case InheritanceSplitExample.BaseInput end split; + return $IsSpecialized; end; / diff --git a/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl b/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl index 9c443a5d8..64d1c7cea 100644 --- a/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl +++ b/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl @@ -23,6 +23,13 @@ -- Validation: -- `mxcli check` parses the script. -- `mx check` against the resulting MPR reports 0 errors. +-- +-- The base-type case (`case BugTest475.Vehicle`) is required for that: an +-- object-type decision needs an outgoing flow for every listed type, and +-- without the base entity the build fails CE0090 regardless of this bug. +-- It is deliberately a TERMINATING branch — the scenario under test is +-- "exactly ONE non-split branch continues", and giving Vehicle a falling +-- -through body would make two branches continue and lose the regression. -- Roundtrip (describe → exec → describe) preserves the structure -- byte-for-byte: the post-split log activity stays outside both case -- bodies. @@ -66,6 +73,8 @@ begin case BugTest475.Boat log info node 'BugTest475' 'Dispatching boat'; return false; + case BugTest475.Vehicle + return false; end split; log info node 'BugTest475' 'Dispatched'; return true; diff --git a/mdl-examples/bug-tests/831-xpath-variable-traversal-ok.mdl b/mdl-examples/bug-tests/831-xpath-variable-traversal-ok.mdl new file mode 100644 index 000000000..59285c468 --- /dev/null +++ b/mdl-examples/bug-tests/831-xpath-variable-traversal-ok.mdl @@ -0,0 +1,56 @@ +-- ============================================================================ +-- Issue #831 — the forms MDL055 must NOT reject (positive half) +-- ============================================================================ +-- +-- The negative half is 831-xpath-variable-traversal.fail.mdl. This file pins +-- the other edge: the two restructurings MDL055's message recommends, plus the +-- one-hop forms that are valid XPath and must not be flagged. +-- +-- Verified against mxbuild 11.6.6: this file builds with 0 errors. +-- ============================================================================ + +CREATE MODULE Bug831Ok; + +CREATE OR MODIFY PERSISTENT ENTITY Bug831Ok.Category ( Name: String(50) ); + +CREATE OR MODIFY PERSISTENT ENTITY Bug831Ok.Product ( Code: String(50) ); + +CREATE OR MODIFY ASSOCIATION Bug831Ok.Product_Category + FROM Bug831Ok.Product TO Bug831Ok.Category TYPE Reference; + +-- Recommended form 1: retrieve the associated object first (one hop is a legal +-- retrieve SOURCE), then constrain on that variable's own attribute. +CREATE OR MODIFY MICROFLOW Bug831Ok.Form1 ($RefProduct: Bug831Ok.Product) +RETURNS list of Bug831Ok.Category +BEGIN + retrieve $Related from $RefProduct/Bug831Ok.Product_Category; + retrieve $Categories from Bug831Ok.Category where [Name = $Related/Name]; + return $Categories; +END; + +-- Recommended form 2: invert the constraint so the traversal starts at the +-- entity being retrieved, which XPath does support. +CREATE OR MODIFY MICROFLOW Bug831Ok.Form2 ($RefProduct: Bug831Ok.Product) +RETURNS list of Bug831Ok.Category +BEGIN + retrieve $Categories from Bug831Ok.Category + where [Bug831Ok.Product_Category/Bug831Ok.Product = $RefProduct]; + return $Categories; +END; + +-- One hop off a variable is valid and must not be flagged: an attribute… +CREATE OR MODIFY MICROFLOW Bug831Ok.OneHopAttribute ($RefProduct: Bug831Ok.Product) +RETURNS list of Bug831Ok.Category +BEGIN + retrieve $Categories from Bug831Ok.Category where [Name = $RefProduct/Code]; + return $Categories; +END; + +-- …and the associated object itself. +CREATE OR MODIFY MICROFLOW Bug831Ok.OneHopAssociation ($RefProduct: Bug831Ok.Product) +RETURNS list of Bug831Ok.Product +BEGIN + retrieve $Products from Bug831Ok.Product + where [Bug831Ok.Product_Category = $RefProduct/Bug831Ok.Product_Category]; + return $Products; +END; diff --git a/mdl-examples/bug-tests/831-xpath-variable-traversal.fail.mdl b/mdl-examples/bug-tests/831-xpath-variable-traversal.fail.mdl new file mode 100644 index 000000000..efebccd60 --- /dev/null +++ b/mdl-examples/bug-tests/831-xpath-variable-traversal.fail.mdl @@ -0,0 +1,44 @@ +-- ============================================================================ +-- Issue #831 — RETRIEVE WHERE traversing an association from a variable +-- ============================================================================ +-- +-- NEGATIVE TEST (.fail.mdl): `mxcli check` MUST reject this file. An +-- unexpected pass means MDL055 has regressed. +-- +-- `where [Name = $RefProduct/ZKT39.Product_Category/Name]` passed `mxcli check` +-- and `mxcli exec`, then the build failed: +-- +-- [error] [CE0161] "Error(s) in XPath constraint." +-- +-- Mendix XPath reaches at most ONE hop off a variable. The boundary is narrower +-- than "a qualified name after a variable" — verified against mxbuild 11.6.6: +-- +-- $Var/Attr VALID the parameter's own attribute +-- $Var/Mod.Assoc VALID one hop, the associated object +-- $Var/Mod.Assoc/Attr CE0161 two or more hops +-- +-- so the rule keys on hop count. A rule that flagged any qualified segment +-- would reject the middle form, which is valid. +-- +-- There is no valid serialization of the two-hop form, which is why this is a +-- rejection and not a writer fix: the constraint has to be restructured, and +-- only the author knows which shape they meant. Both restructurings are in +-- 831-xpath-variable-traversal-ok.mdl, which must PASS. +-- ============================================================================ + +CREATE MODULE Bug831; + +CREATE OR MODIFY PERSISTENT ENTITY Bug831.Category ( Name: String(50) ); + +CREATE OR MODIFY PERSISTENT ENTITY Bug831.Product ( Code: String(50) ); + +CREATE OR MODIFY ASSOCIATION Bug831.Product_Category + FROM Bug831.Product TO Bug831.Category TYPE Reference; + +CREATE OR MODIFY MICROFLOW Bug831.ACT_Find ($RefProduct: Bug831.Product) +RETURNS list of Bug831.Category +BEGIN + retrieve $Categories from Bug831.Category + where [Name = $RefProduct/Bug831.Product_Category/Name]; + return $Categories; +END; diff --git a/mdl-examples/bug-tests/832-npe-validation-rules-ok.mdl b/mdl-examples/bug-tests/832-npe-validation-rules-ok.mdl new file mode 100644 index 000000000..7e7c088cc --- /dev/null +++ b/mdl-examples/bug-tests/832-npe-validation-rules-ok.mdl @@ -0,0 +1,24 @@ +-- ============================================================================ +-- Issue #832 — the forms MDL054 must NOT reject (positive half) +-- ============================================================================ +-- +-- The negative half is 832-npe-validation-rules.fail.mdl. This file pins the +-- other edge: MDL054 must not fire on a validation rule that is legitimately +-- placed, or on a non-persistent entity that carries none. +-- +-- Verified against mxbuild 11.6.6: this file builds with 0 errors. +-- ============================================================================ + +CREATE MODULE Bug832Ok; + +-- A PERSISTENT entity is exactly where validation rules belong. +CREATE OR MODIFY PERSISTENT ENTITY Bug832Ok.P ( + "Name": String(100) not null error 'Name is required', + "Code": String(50) unique error 'Code must be unique' +); + +-- A non-persistent entity with no validation rule is fine. +CREATE OR MODIFY NON-PERSISTENT ENTITY Bug832Ok.NpPlain ( + "Name": String(100), + "Qty": Integer +); diff --git a/mdl-examples/bug-tests/832-npe-validation-rules.fail.mdl b/mdl-examples/bug-tests/832-npe-validation-rules.fail.mdl new file mode 100644 index 000000000..dba2caf20 --- /dev/null +++ b/mdl-examples/bug-tests/832-npe-validation-rules.fail.mdl @@ -0,0 +1,36 @@ +-- ============================================================================ +-- Issue #832 — validation rules on a non-persistent entity were accepted +-- ============================================================================ +-- +-- NEGATIVE TEST (.fail.mdl): `mxcli check` MUST reject this file. An +-- unexpected pass means MDL054 has regressed. +-- +-- Mendix refuses a validation rule on a non-persistable entity: +-- +-- [error] [CE0070] "Validations rules are not allowed on entity 'X', +-- because it is not persistable." +-- +-- `not null` and `unique` ARE validation rules — Studio Pro models "required" +-- and "uniqueness" as rules on the entity, not as column constraints — so both +-- are rejected on an NPE. `mxcli check` and `mxcli exec` both accepted them and +-- only a real build caught it, which is the worst place to find out. +-- +-- Verified against mxbuild 11.6.6: `not null` with a message, `not null` bare, +-- and `unique` each produce CE0070; a plain attribute does not. The message is +-- optional and does not change the verdict. +-- +-- The accepted counterparts — the same constraints on a PERSISTENT entity, and +-- an NPE with no constraint — are in 832-npe-validation-rules-ok.mdl, which +-- must PASS. Together they pin both edges of the rule. +-- +-- Only the CREATE path can catch this: an `ALTER ENTITY … ADD ATTRIBUTE` does +-- not carry the entity's persistence kind, so it cannot be told apart from a +-- persistent entity without a project. Same limitation as MDL020. +-- ============================================================================ + +CREATE MODULE Bug832; + +CREATE OR MODIFY NON-PERSISTENT ENTITY Bug832.Np ( + "Name": String(100) not null error 'Name is required', + "Code": String(50) unique error 'Code must be unique' +); diff --git a/mdl-examples/bug-tests/834-alter-customcontent-column-widget.mdl b/mdl-examples/bug-tests/834-alter-customcontent-column-widget.mdl new file mode 100644 index 000000000..b44f3ad36 --- /dev/null +++ b/mdl-examples/bug-tests/834-alter-customcontent-column-widget.mdl @@ -0,0 +1,56 @@ +-- ============================================================================ +-- Issue #834 — ALTER PAGE could not reach a widget inside a customContent column +-- ============================================================================ +-- +-- `alter page … set Caption = '…' on btnEdit` reported "widget btnEdit not +-- found" when btnEdit lived inside a datagrid column rendered as +-- customContent. The only remedy was CREATE OR REPLACE PAGE — a full rewrite. +-- +-- Two independent defects were in play: +-- +-- 1. The resolver never descended into a column's own content. Columns live +-- at Object.Properties[columns].Value.Objects[]; each column's widgets are +-- one level deeper at Properties[content].Value.Widgets[]. The pluggable +-- search only reached the grid's own Object.Properties[].Value.Widgets. +-- +-- 2. `set Caption` failed on EVERY action button, nested or top-level, with +-- "widget has no Caption property" — an ActionButton stores its caption as +-- a Forms$ClientTemplate under CaptionTemplate, not as a Caption document. +-- +-- Addressing is by the nested widget's OWN name. A `grid.column.widget` path +-- was considered and rejected: DataGrid2 columns carry no stored name in the +-- MPR, so the column segment could only ever be a derived name (the bound +-- attribute, or the caption) — which changes when the caption is edited, making +-- such a path silently stale. The nested widget's name is real and stable. +-- +-- To verify: +-- 1. Run this script. +-- 2. `mxcli describe page Bug834.PgCC` — btnEdit's caption must read +-- 'Changed' (nested) and btnTop's must read 'Changed too' (top-level). +-- 3. mx check reports 0 errors (verified on 11.6.6). +-- ============================================================================ + +CREATE MODULE Bug834; + +CREATE OR MODIFY PERSISTENT ENTITY Bug834.Thing ( Slug: String(50) ); + +CREATE OR REPLACE PAGE Bug834.PgCC ( Title: 'PgCC', Layout: Atlas_Core.Atlas_Default ) +{ + actionbutton btnTop (caption: 'Edit') + datagrid dgcc (datasource: database from Bug834.Thing) { + column colA (attribute: Slug, caption: 'Slug') + column colB (caption: 'Act', ShowContentAs: customContent) { + actionbutton btnEdit (caption: 'Edit') + } + } +} + +-- Defect 1: reaching a widget nested in a customContent column. +ALTER PAGE Bug834.PgCC { + set caption = 'Changed' on btnEdit; +}; + +-- Defect 2: the same setter on a top-level action button, which failed too. +ALTER PAGE Bug834.PgCC { + set caption = 'Changed too' on btnTop; +}; diff --git a/mdl-examples/bug-tests/mdl009-enum-split-empty-branch-ok.mdl b/mdl-examples/bug-tests/mdl009-enum-split-empty-branch-ok.mdl new file mode 100644 index 000000000..0a759a309 --- /dev/null +++ b/mdl-examples/bug-tests/mdl009-enum-split-empty-branch-ok.mdl @@ -0,0 +1,59 @@ +-- ============================================================================ +-- MDL009 retired / MDL056 — the enum-split forms that must NOT be rejected +-- ============================================================================ +-- +-- The negative half is mdl009-enum-split-empty-branch.fail.mdl. +-- +-- Verified against mxbuild 11.6.6: this file builds with 0 errors, including +-- the multi-value branch that the retired MDL009 used to reject. +-- ============================================================================ + +CREATE MODULE BugM9Ok; + +CREATE ENUMERATION BugM9Ok.Status ( + Open caption 'Open', + Pending caption 'Pending', + Closed caption 'Closed' +); + +-- A multi-value branch is valid — this is what MDL009 wrongly rejected. +CREATE OR MODIFY MICROFLOW BugM9Ok.MultiValue ($Status: Enumeration(BugM9Ok.Status)) +RETURNS Boolean +BEGIN + case $Status + when Open, Pending then + return true; + when Closed then + return false; + when (empty) then + return false; + end case; +END; + +-- One value per branch is equally valid. +CREATE OR MODIFY MICROFLOW BugM9Ok.OnePerBranch ($Status: Enumeration(BugM9Ok.Status)) +RETURNS Boolean +BEGIN + case $Status + when Open then + return true; + when Pending then + return true; + when Closed then + return false; + when (empty) then + return false; + end case; +END; + +-- `(empty)` may share a branch with real values. +CREATE OR MODIFY MICROFLOW BugM9Ok.EmptySharesBranch ($Status: Enumeration(BugM9Ok.Status)) +RETURNS Boolean +BEGIN + case $Status + when Open, Pending then + return true; + when Closed, (empty) then + return false; + end case; +END; diff --git a/mdl-examples/bug-tests/mdl009-enum-split-empty-branch.fail.mdl b/mdl-examples/bug-tests/mdl009-enum-split-empty-branch.fail.mdl new file mode 100644 index 000000000..2cc6140ed --- /dev/null +++ b/mdl-examples/bug-tests/mdl009-enum-split-empty-branch.fail.mdl @@ -0,0 +1,43 @@ +-- ============================================================================ +-- MDL009 retired, MDL056 added — enum split branch rules +-- ============================================================================ +-- +-- NEGATIVE TEST (.fail.mdl): `mxcli check` MUST reject this file. +-- +-- MDL009 used to error on `when Open, Pending then`, claiming Mendix required +-- exactly one value per branch. That was wrong — verified on mxbuild 11.6.6, a +-- multi-value branch covering every value builds with 0 errors, and the shipped +-- write-microflows skill documents that form. The rule rejected valid MDL, so +-- it is retired. +-- +-- What actually fails the build is a MISSING branch. An enum split is an +-- exclusive split needing one outgoing flow per condition value: +-- +-- [error] [CE0079] "The '(empty)' condition value should be configured in +-- properties for an outgoing flow." +-- +-- MDL056 catches the `(empty)` case, which is universal and needs no knowledge +-- of the enumeration's members — it holds even on a `not null` attribute. +-- +-- The valid forms are in mdl009-enum-split-empty-branch-ok.mdl, which must PASS. +-- ============================================================================ + +CREATE MODULE BugM9; + +CREATE ENUMERATION BugM9.Status ( + Open caption 'Open', + Pending caption 'Pending', + Closed caption 'Closed' +); + +-- REJECTED (MDL056): every value is covered, but `(empty)` is not. +CREATE OR MODIFY MICROFLOW BugM9.NoEmptyBranch ($Status: Enumeration(BugM9.Status)) +RETURNS Boolean +BEGIN + case $Status + when Open, Pending then + return true; + when Closed then + return false; + end case; +END; diff --git a/mdl-examples/bug-tests/split-type-dangling-pointer.mdl b/mdl-examples/bug-tests/split-type-dangling-pointer.mdl new file mode 100644 index 000000000..7b42908db --- /dev/null +++ b/mdl-examples/bug-tests/split-type-dangling-pointer.mdl @@ -0,0 +1,47 @@ +-- ============================================================================ +-- `split type` wrote a project mxbuild could not LOAD +-- ============================================================================ +-- +-- `mxcli check` passed and `mxcli exec` reported success, but `mx check` died +-- before validating anything: +-- +-- ERROR: System.Collections.Generic.KeyNotFoundException: The given key +-- '' was not present in the dictionary +-- at StreamingBsonUnitReader.ResolvePostponedProperties() +-- +-- Reproduced on Mendix 11.6.6 and 11.13.0. Two gaps in the modelsdk writer, +-- both the #791 shape — an object dropped at serialization while the sequence +-- flows pointing at it were still written: +-- +-- 1. microflowObjectToGen had no *microflows.InheritanceSplit case, so the +-- split itself vanished. Three flows referenced its $ID. +-- 2. caseValueToGen had no InheritanceCase case, so every branch degraded to +-- a bare Microflows$NoCase and lost the entity it selects on. +-- +-- Diagnosed with the #791 recipe: dump the microflow, collect every $ID, and +-- check each key ending in `Pointer` resolves. Before: 27 objects, 10 pointers, +-- 3 dangling. After: 28 objects, 10 pointers, 0 dangling. +-- +-- A type split must give every type an outgoing flow, INCLUDING the base type +-- (CE0090 otherwise). An `else` does NOT substitute for the base-type case — +-- verified on both versions. +-- +-- To verify: run this script, then `mx check` — 0 errors. +-- ============================================================================ + +CREATE MODULE BugSplit; + +CREATE OR MODIFY PERSISTENT ENTITY BugSplit.Animal ( Name: String(50) ); + +CREATE OR MODIFY PERSISTENT ENTITY BugSplit.Dog EXTENDS BugSplit.Animal ( Breed: String(50) ); + +CREATE OR MODIFY MICROFLOW BugSplit.Classify ($A: BugSplit.Animal) +RETURNS String +BEGIN + split type $A + case BugSplit.Dog + cast $d; + case BugSplit.Animal + end split; + return 'done'; +END; diff --git a/mdl-examples/doctype-tests/18-folder-examples.mdl b/mdl-examples/doctype-tests/18-folder-examples.mdl index bc2626cc2..06afa13fc 100644 --- a/mdl-examples/doctype-tests/18-folder-examples.mdl +++ b/mdl-examples/doctype-tests/18-folder-examples.mdl @@ -89,4 +89,65 @@ drop folder 'Resources' in FolderTest; / -- cleanup +-- ============================================================================ +-- Level 6: Java actions and published OData services +-- ============================================================================ + +/** + * Neither CREATE JAVA ACTION nor CREATE ODATA SERVICE takes a folder clause, so + * MOVE is the only way these documents ever leave the module root. + */ +create java action FolderTest.QueryHelper () returns Boolean as $$ +public class QueryHelper { } +$$; + +move java action FolderTest.QueryHelper to folder 'Support/Java'; + +create non-persistent entity FolderTest.ApiRow ( RowKey: string(60) ); + +CREATE MICROFLOW FolderTest.Read_ApiRows () + RETURNS List of FolderTest.ApiRow AS $Rows +BEGIN + $Rows = CREATE LIST OF FolderTest.ApiRow; + RETURN $Rows; +END; + +create odata service FolderTest.PublicApi ( + path: 'odata/foldertest/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'FolderTest.PublicApi', + ServiceName: 'PublicApi' +) +{ + publish entity FolderTest.ApiRow as 'ApiRows' ( + ReadMode: microflow FolderTest.Read_ApiRows, + InsertMode: not_supported, + UpdateMode: not_supported, + DeleteMode: not_supported, + Countable: false + ) + expose ( RowKey as 'rowKey' (KEY) ); +}; + +move odata service FolderTest.PublicApi to folder 'Api/Published'; + +-- ============================================================================ +-- Level 7: Read the layout back +-- ============================================================================ + +/** + * LIST FOLDERS is the counterpart to MOVE: it shows the folder layout and what + * sits in each folder, including empty folders and anything still at the module + * root. SHOW STRUCTURE groups by document type at every depth and never shows + * which folder a document is in. + */ +list folders in FolderTest; + +-- SHOW is accepted as the legacy verb for the same statement. +show folders in FolderTest; + +-- With no IN clause, every module in the project. +list folders; + drop module FolderTest; diff --git a/mdl-examples/doctype-tests/cleanup-rollback.test.mdl b/mdl-examples/doctype-tests/cleanup-rollback.test.mdl new file mode 100644 index 000000000..204177837 --- /dev/null +++ b/mdl-examples/doctype-tests/cleanup-rollback.test.mdl @@ -0,0 +1,56 @@ +-- ============================================================================ +-- @cleanup rollback — worked example +-- ============================================================================ +-- Demonstrates what happens to a test's database writes. +-- +-- @cleanup rollback (the default) the writes are rolled back +-- @cleanup none the writes commit and persist +-- +-- Rollback needs the test endpoint, which owns the context each test runs in, +-- so it applies to `--local` and `--attach`. The Docker / --legacy-runner path +-- runs tests inside the after-startup action and always commits. +-- +-- Setup — these are the microflows under test: +-- +-- create persistent entity App.Person (FirstName: string(100)); +-- +-- create microflow App.CreatePerson (FirstName: string) +-- returns string as $Stored +-- begin +-- declare $Stored String = ''; +-- $P = create App.Person (FirstName = $FirstName); +-- commit $P; +-- set $Stored = $P/FirstName; +-- return $Stored; +-- end; +-- / +-- +-- Run: mxcli test cleanup-rollback.test.mdl -p app.mpr --local --verbose +-- +-- --verbose tags each result [rolled back] / [committed], and afterwards only +-- the PersistedProbe row is in the database. +-- ============================================================================ + +/** + * @test the default is rollback — this Person does not survive the run + * @expect $result = 'RollbackProbe' + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'RollbackProbe'); +/ + +/** + * @test stating rollback explicitly does the same thing + * @expect $result = 'ExplicitRollbackProbe' + * @cleanup rollback + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'ExplicitRollbackProbe'); +/ + +/** + * @test @cleanup none commits — the control that proves rollback is doing the + * work above, and the way to seed a fixture you want to keep + * @expect $result = 'PersistedProbe' + * @cleanup none + */ +$result = CALL MICROFLOW App.CreatePerson(FirstName = 'PersistedProbe'); +/ diff --git a/mdl/ast/ast.go b/mdl/ast/ast.go index dca56fb91..256d2cfd3 100644 --- a/mdl/ast/ast.go +++ b/mdl/ast/ast.go @@ -54,6 +54,8 @@ const ( DocumentTypeEnumeration DocumentType = "ENUMERATION" DocumentTypeConstant DocumentType = "CONSTANT" DocumentTypeDatabaseConnection DocumentType = "DATABASE CONNECTION" + DocumentTypeJavaAction DocumentType = "JAVA ACTION" + DocumentTypeODataService DocumentType = "ODATA SERVICE" ) // MoveStmt represents: MOVE PAGE/MICROFLOW/SNIPPET/NANOFLOW/ENTITY/ENUMERATION Module.Name TO FOLDER 'path' IN Module diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index ac4f48b4d..3f7af75e2 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -452,13 +452,18 @@ func (s *CallWebServiceStmt) isMicroflowStatement() {} // ExecuteDatabaseQueryStmt represents: EXECUTE DATABASE QUERY Module.Connection.QueryName ... type ExecuteDatabaseQueryStmt struct { - OutputVariable string // Optional output variable - QueryName string // Full 3-part identifier: Module.Connection.QueryName - DynamicQuery string // Optional dynamic SQL override - Arguments []CallArgument // Parameter mappings (query parameters) - ConnectionArguments []CallArgument // Connection parameter mappings (runtime connection override) - ErrorHandling *ErrorHandlingClause // Optional ON ERROR clause - Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation + OutputVariable string // Optional output variable + QueryName string // Full 3-part identifier: Module.Connection.QueryName + DynamicQuery string // Optional dynamic SQL override + // DynamicQueryIsExpression distinguishes `dynamic $Sql` from `dynamic 'SELECT …'`. + // Both reach the executor as a bare string, and the builder has to quote one + // and not the other: quoting an expression sends the literal text `$Sql` to + // the database, which is a syntax error at the far end, not a Mendix one. + DynamicQueryIsExpression bool + Arguments []CallArgument // Parameter mappings (query parameters) + ConnectionArguments []CallArgument // Connection parameter mappings (runtime connection override) + ErrorHandling *ErrorHandlingClause // Optional ON ERROR clause + Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation } func (s *ExecuteDatabaseQueryStmt) isMicroflowStatement() {} diff --git a/mdl/ast/ast_odata.go b/mdl/ast/ast_odata.go index 729a8a24a..158129ab7 100644 --- a/mdl/ast/ast_odata.go +++ b/mdl/ast/ast_odata.go @@ -24,7 +24,15 @@ type CreateODataClientStmt struct { UseAuthentication bool HttpUsername string // Mendix expression for username HttpPassword string // Mendix expression for password - ClientCertificate string + + // Whether the credential above was written as a quoted literal rather than + // a constant reference. The visitor strips a literal's quotes, so by the + // time it reaches the executor `'f1api'` and `Module.ApiUser` are both bare + // strings — and only the first is a value mxcli can use for the design-time + // $metadata fetch. A constant is resolved by the runtime, not by us. + HttpUsernameIsLiteral bool + HttpPasswordIsLiteral bool + ClientCertificate string // Microflow references. `ConfigurationMicroflow` (returns // System.ConsumedODataConfiguration) and `HeadersMicroflow` (returns a list @@ -52,6 +60,9 @@ type CreateODataClientStmt struct { type HeaderDef struct { Key string Value string // Mendix expression + // ValueIsLiteral mirrors HttpUsernameIsLiteral: a quoted literal can be sent + // on the design-time fetch, a constant reference cannot. + ValueIsLiteral bool } func (s *CreateODataClientStmt) isStatement() {} diff --git a/mdl/ast/ast_query.go b/mdl/ast/ast_query.go index 56bf540f0..d2b80be1f 100644 --- a/mdl/ast/ast_query.go +++ b/mdl/ast/ast_query.go @@ -24,6 +24,7 @@ type ShowObjectType int const ( ShowModules ShowObjectType = iota ShowEnumerations + ShowFolders ShowConstants ShowEntities ShowEntity @@ -110,6 +111,8 @@ func (t ShowObjectType) String() string { return "MODULES" case ShowEnumerations: return "ENUMERATIONS" + case ShowFolders: + return "FOLDERS" case ShowConstants: return "CONSTANTS" case ShowEntities: diff --git a/mdl/ast/ast_security.go b/mdl/ast/ast_security.go index 09e06cdf2..9c03d99e7 100644 --- a/mdl/ast/ast_security.go +++ b/mdl/ast/ast_security.go @@ -10,6 +10,10 @@ package ast type CreateModuleRoleStmt struct { Name QualifiedName Description string + // CreateOrModify makes the statement idempotent: an existing role has its + // description updated instead of the statement failing, so a security script + // can be re-run. + CreateOrModify bool } func (s *CreateModuleRoleStmt) isStatement() {} diff --git a/mdl/backend/java.go b/mdl/backend/java.go index 73a3e5d72..b906eabb9 100644 --- a/mdl/backend/java.go +++ b/mdl/backend/java.go @@ -12,6 +12,8 @@ import ( type JavaBackend interface { ListJavaActions() ([]*types.JavaAction, error) ListJavaActionsFull() ([]*javaactions.JavaAction, error) + // MoveJavaAction reparents a Java action to an already-updated ContainerID. + MoveJavaAction(ja *javaactions.JavaAction) error ListJavaScriptActions() ([]*types.JavaScriptAction, error) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaAction, error) ReadJavaScriptActionByName(qualifiedName string) (*types.JavaScriptAction, error) diff --git a/mdl/backend/mcp/unsupported_gen.go b/mdl/backend/mcp/unsupported_gen.go index 35878e3b9..637e5e5d6 100644 --- a/mdl/backend/mcp/unsupported_gen.go +++ b/mdl/backend/mcp/unsupported_gen.go @@ -667,13 +667,13 @@ func (unsupportedBackend) ListFolders() (r0 []*types.FolderInfo, err1 error) { return } -func (unsupportedBackend) ListImageCollections() (r0 []*types.ImageCollection, err1 error) { - err1 = errUnsupported("ListImageCollections") +func (unsupportedBackend) ListIconCollections() (r0 []*types.IconCollection, err1 error) { + err1 = errUnsupported("ListIconCollections") return } -func (unsupportedBackend) ListIconCollections() (r0 []*types.IconCollection, err1 error) { - err1 = errUnsupported("ListIconCollections") +func (unsupportedBackend) ListImageCollections() (r0 []*types.ImageCollection, err1 error) { + err1 = errUnsupported("ListImageCollections") return } @@ -827,6 +827,11 @@ func (unsupportedBackend) MoveImportMapping(_ *model.ImportMapping) (err0 error) return } +func (unsupportedBackend) MoveJavaAction(_ *javaactions.JavaAction) (err0 error) { + err0 = errUnsupported("MoveJavaAction") + return +} + func (unsupportedBackend) MoveMicroflow(_ *microflows.Microflow) (err0 error) { err0 = errUnsupported("MoveMicroflow") return @@ -842,6 +847,11 @@ func (unsupportedBackend) MovePage(_ *pages.Page) (err0 error) { return } +func (unsupportedBackend) MovePublishedODataService(_ *model.PublishedODataService) (err0 error) { + err0 = errUnsupported("MovePublishedODataService") + return +} + func (unsupportedBackend) MoveSnippet(_ *pages.Snippet) (err0 error) { err0 = errUnsupported("MoveSnippet") return diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index 7930593ef..b2d9e8c62 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -185,6 +185,8 @@ type MockBackend struct { CreateDatabaseConnectionFunc func(conn *model.DatabaseConnection) error UpdateDatabaseConnectionFunc func(conn *model.DatabaseConnection) error MoveDatabaseConnectionFunc func(conn *model.DatabaseConnection) error + MoveJavaActionFunc func(ja *javaactions.JavaAction) error + MovePublishedODataServiceFunc func(svc *model.PublishedODataService) error DeleteDatabaseConnectionFunc func(id model.ID) error ListDataTransformersFunc func() ([]*model.DataTransformer, error) CreateDataTransformerFunc func(dt *model.DataTransformer) error diff --git a/mdl/backend/mock/mock_service.go b/mdl/backend/mock/mock_service.go index 6dbf0cfd1..cb37e74fd 100644 --- a/mdl/backend/mock/mock_service.go +++ b/mdl/backend/mock/mock_service.go @@ -2,7 +2,12 @@ package mock -import "github.com/mendixlabs/mxcli/model" +import ( + "errors" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" +) func (m *MockBackend) ListConsumedODataServices() ([]*model.ConsumedODataService, error) { if m.ListConsumedODataServicesFunc != nil { @@ -172,6 +177,20 @@ func (m *MockBackend) MoveDatabaseConnection(conn *model.DatabaseConnection) err return nil } +func (m *MockBackend) MoveJavaAction(ja *javaactions.JavaAction) error { + if m.MoveJavaActionFunc != nil { + return m.MoveJavaActionFunc(ja) + } + return errors.New("MockBackend.MoveJavaAction not configured") +} + +func (m *MockBackend) MovePublishedODataService(svc *model.PublishedODataService) error { + if m.MovePublishedODataServiceFunc != nil { + return m.MovePublishedODataServiceFunc(svc) + } + return errors.New("MockBackend.MovePublishedODataService not configured") +} + func (m *MockBackend) DeleteDatabaseConnection(id model.ID) error { if m.DeleteDatabaseConnectionFunc != nil { return m.DeleteDatabaseConnectionFunc(id) diff --git a/mdl/backend/modelsdk/domainmodel.go b/mdl/backend/modelsdk/domainmodel.go index a8d61fa80..23260e7e7 100644 --- a/mdl/backend/modelsdk/domainmodel.go +++ b/mdl/backend/modelsdk/domainmodel.go @@ -266,6 +266,24 @@ func attributeFromGen(a *genDm.Attribute) *domainmodel.Attribute { // View-entity attribute: the OQL column reference must survive a // read-modify-write (e.g. MOVE ENTITY) or the view goes out of sync (CE6770). attr.Value = &domainmodel.AttributeValue{ViewReference: v.Reference()} + case *genRest.ODataMappedValue: + // External-entity attribute: the mapping to the remote OData property. + // Reading it back is what makes a read-modify-write safe — without it + // every attribute of an external entity comes back unmapped, and the + // writer's `isExternal && a.RemoteName != ""` arm falls through to a + // plain StoredValue. The entity then no longer matches the contract: + // "Attribute 'year' of external entity 'Stg_Season' is not supported." + attr.RemoteName = v.RemoteName() + attr.RemoteType = v.RemoteType() + attr.Filterable = v.Filterable() + attr.Sortable = v.Sortable() + attr.Creatable = v.Creatable() + attr.Updatable = v.Updatable() + case *genRest.ODataMappedPrimitiveCollectionValue: + // The single attribute of a primitive-collection NPE (issue #718). + attr.RemoteName = v.RemoteName() + attr.RemoteType = v.RemoteType() + attr.IsPrimitiveCollection = true } return attr } diff --git a/mdl/backend/modelsdk/external_entity_read_test.go b/mdl/backend/modelsdk/external_entity_read_test.go index 556773014..3c737c9fb 100644 --- a/mdl/backend/modelsdk/external_entity_read_test.go +++ b/mdl/backend/modelsdk/external_entity_read_test.go @@ -227,3 +227,49 @@ func TestExternalEntity_PrimitiveCollectionSourceRoundTrip(t *testing.T) { t.Errorf("RemoteServiceName = %q", got.RemoteServiceName) } } + +// TestExternalEntity_AttributeRemoteMappingRoundTrip is the attribute-level half +// of #782, found by mxcli-formula1 #25: entityFromGen learned to read the +// entity's own remote fields, but attributeFromGen still handled only +// StoredValue and OqlViewValue. A Rest$ODataMappedValue therefore came back with +// no RemoteName, and the write path's `isExternal && a.RemoteName != ""` arm fell +// through to a plain StoredValue on the next read-modify-write. +// +// The visible failure is a `create or modify external entity` that touches only +// an entity-level property and detonates every attribute: +// +// [CE6612] "Attribute 'circuitId' of external entity 'Stg_Circuit' is not supported." +// +// one per attribute. Confirmed on 11.12.1 against a real contract import: three +// CE6612 before the fix, none after. +func TestExternalEntity_AttributeRemoteMappingRoundTrip(t *testing.T) { + proj, modID := externalEntityFixture(t, func(e *domainmodel.Entity) { + e.Attributes = []*domainmodel.Attribute{{ + Name: "ProductName", + Type: &domainmodel.StringAttributeType{Length: 120}, + RemoteName: "Name", + RemoteType: "Edm.String", + Filterable: true, + Sortable: true, + Updatable: true, + }} + }) + got := readEntity(t, proj, modID, "Products") + + if len(got.Attributes) != 1 { + t.Fatalf("got %d attributes, want 1", len(got.Attributes)) + } + a := got.Attributes[0] + if a.RemoteName != "Name" { + t.Errorf("RemoteName = %q, want Name — the OData mapping did not survive the read", a.RemoteName) + } + if a.RemoteType != "Edm.String" { + t.Errorf("RemoteType = %q, want Edm.String", a.RemoteType) + } + // The per-attribute capability flags live on the same ODataMappedValue and + // are equally lost if the case arm is missing. + if !a.Filterable || !a.Sortable || !a.Updatable { + t.Errorf("capability flags lost: filterable=%v sortable=%v updatable=%v", + a.Filterable, a.Sortable, a.Updatable) + } +} diff --git a/mdl/backend/modelsdk/microflow_inheritance_write_test.go b/mdl/backend/modelsdk/microflow_inheritance_write_test.go new file mode 100644 index 000000000..50f9b1892 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_inheritance_write_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// TestMicroflowRoundTrip_InheritanceSplit covers a corruption found while +// testing enum-split `else` across versions: `split type` produced a project +// mxbuild could not LOAD at all — +// +// KeyNotFoundException: The given key '' was not present in the +// dictionary at StreamingBsonUnitReader.ResolvePostponedProperties() +// +// on both 11.6.6 and 11.13.0, while `mxcli check` passed. Two gaps, both the +// #791 shape (an object dropped at serialization while the flows pointing at +// it were written): +// +// 1. microflowObjectToGen had no *microflows.InheritanceSplit case, so the +// split hit `default: return nil` and vanished. Three sequence flows +// referenced its $ID — that is the dangling pointer the loader trips on. +// 2. caseValueToGen had no InheritanceCase case, so every branch flow got a +// bare NoCase and the entity each branch selects on was lost. +func TestMicroflowRoundTrip_InheritanceSplit(t *testing.T) { + split := µflows.InheritanceSplit{VariableName: "A", Caption: "split"} + split.ID = model.ID("split-1") + + mf := µflows.Microflow{ + Name: "TypeSplit", + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{split}, + }, + } + mf.ID = model.ID("mf-1") + + got := roundTripMicroflow(t, mf) + + var found *microflows.InheritanceSplit + if got.ObjectCollection != nil { + for _, obj := range got.ObjectCollection.Objects { + if s, ok := obj.(*microflows.InheritanceSplit); ok { + found = s + } + } + } + if found == nil { + t.Fatal("InheritanceSplit did not survive the round trip — the object is dropped at " + + "serialization while flows still point at its $ID, which is the KeyNotFoundException") + } + if found.VariableName != "A" { + t.Errorf("VariableName = %q, want A", found.VariableName) + } +} + +// The branch's case value must round-trip as an InheritanceCase naming the +// entity, not degrade to a NoCase. +func TestCaseValueToGen_InheritanceCase(t *testing.T) { + el := caseValueToGen(µflows.InheritanceCase{EntityQualifiedName: "SP.Dog"}) + if el == nil { + t.Fatal("caseValueToGen returned nil for an InheritanceCase") + } + if got := el.TypeName(); got != "Microflows$InheritanceCase" { + t.Fatalf("$Type = %q, want Microflows$InheritanceCase (a NoCase loses the branch entity)", got) + } +} + +// The visitor sometimes yields value receivers; those must dispatch the same +// way, exactly as the existing normalisation does for EnumerationCase. +func TestCaseValueToGen_InheritanceCaseValueReceiver(t *testing.T) { + el := caseValueToGen(microflows.InheritanceCase{EntityQualifiedName: "SP.Dog"}) + if el == nil || el.TypeName() != "Microflows$InheritanceCase" { + t.Fatalf("value-receiver InheritanceCase degraded to %v", el) + } +} diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 05db0d046..d56947cb4 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -307,6 +307,21 @@ func microflowObjectToGen(obj microflows.MicroflowObject) element.Element { g.SetSplitCondition(sc) } return g + case *microflows.InheritanceSplit: + // Without this the split hit `default: return nil` and was dropped, while + // the sequence flows referencing its $ID were still written — a dangling + // pointer that mxbuild cannot even load ("KeyNotFoundException ... at + // StreamingBsonUnitReader.ResolvePostponedProperties"). Same shape as the + // ErrorEvent/BreakEvent gap in #791. Fields mirror the legacy serializer + // in sdk/mpr/writer_microflow.go. + g := genMf.NewInheritanceSplit() + g.SetID(element.ID(o.ID)) + g.SetCaption(o.Caption) + g.SetDocumentation(o.Documentation) + g.SetRelativeMiddlePoint(pointStr(o.Position)) + g.SetSize(sizeStr(o.Size)) + g.SetSplitVariableName(o.VariableName) + return g case *microflows.ExclusiveMerge: g := genMf.NewExclusiveMerge() g.SetID(element.ID(o.ID)) @@ -1087,6 +1102,8 @@ func caseValueToGen(cv microflows.CaseValue) element.Element { cv = &c case microflows.NoCase: cv = &c + case microflows.InheritanceCase: + cv = &c } switch c := cv.(type) { case *microflows.EnumerationCase: @@ -1099,6 +1116,14 @@ func caseValueToGen(cv microflows.CaseValue) element.Element { g.SetID(element.ID(c.ID)) g.SetValue(c.Expression) return g + case *microflows.InheritanceCase: + // A type-split branch selects on an entity. Without this it fell through + // to NoCase, so every branch lost the entity it matches on — the second + // half of the `split type` corruption. + g := genMf.NewInheritanceCase() + g.SetID(element.ID(c.ID)) + g.SetValueQualifiedName(c.EntityQualifiedName) + return g default: return genMf.NewNoCase() } diff --git a/mdl/backend/modelsdk/move_documents_write.go b/mdl/backend/modelsdk/move_documents_write.go index b93afa3cf..9919b7f03 100644 --- a/mdl/backend/modelsdk/move_documents_write.go +++ b/mdl/backend/modelsdk/move_documents_write.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" "github.com/mendixlabs/mxcli/sdk/microflows" "github.com/mendixlabs/mxcli/sdk/pages" ) @@ -69,3 +70,17 @@ func (b *Backend) MoveDatabaseConnection(conn *model.DatabaseConnection) error { } return b.moveUnit(conn.ID, conn.ContainerID, "DatabaseConnection") } + +func (b *Backend) MoveJavaAction(ja *javaactions.JavaAction) error { + if ja == nil { + return fmt.Errorf("MoveJavaAction: nil java action") + } + return b.moveUnit(ja.ID, ja.ContainerID, "JavaAction") +} + +func (b *Backend) MovePublishedODataService(svc *model.PublishedODataService) error { + if svc == nil { + return fmt.Errorf("MovePublishedODataService: nil service") + } + return b.moveUnit(svc.ID, svc.ContainerID, "PublishedODataService") +} diff --git a/mdl/backend/modelsdk/odata_write.go b/mdl/backend/modelsdk/odata_write.go index 7a153da21..a02b7d92b 100644 --- a/mdl/backend/modelsdk/odata_write.go +++ b/mdl/backend/modelsdk/odata_write.go @@ -354,7 +354,7 @@ func publishedMemberToGen(m *model.PublishedMember, ownerQN string) element.Elem addBool(g, "Filterable", m.Filterable) addBool(g, "Sortable", m.Sortable) addBool(g, "IsPartOfKey", m.IsPartOfKey) - addBool(g, "EnumerationAsString", false) + addBool(g, "EnumerationAsString", m.EnumerationAsString) addBool(g, "StringAsGuid", false) return g } diff --git a/mdl/backend/modelsdk/unimplemented_gen.go b/mdl/backend/modelsdk/unimplemented_gen.go index c0a682247..ce24b4208 100644 --- a/mdl/backend/modelsdk/unimplemented_gen.go +++ b/mdl/backend/modelsdk/unimplemented_gen.go @@ -595,6 +595,11 @@ func (unimplemented) ListFolders() ([]*types.FolderInfo, error) { return r0, errUnimplemented("ListFolders") } +func (unimplemented) ListIconCollections() ([]*types.IconCollection, error) { + var r0 []*types.IconCollection + return r0, errUnimplemented("ListIconCollections") +} + func (unimplemented) ListImageCollections() ([]*types.ImageCollection, error) { var r0 []*types.ImageCollection return r0, errUnimplemented("ListImageCollections") @@ -744,6 +749,10 @@ func (unimplemented) MoveImportMapping(_ *model.ImportMapping) error { return errUnimplemented("MoveImportMapping") } +func (unimplemented) MoveJavaAction(_ *javaactions.JavaAction) error { + return errUnimplemented("MoveJavaAction") +} + func (unimplemented) MoveMicroflow(_ *microflows.Microflow) error { return errUnimplemented("MoveMicroflow") } @@ -756,6 +765,10 @@ func (unimplemented) MovePage(_ *pages.Page) error { return errUnimplemented("MovePage") } +func (unimplemented) MovePublishedODataService(_ *model.PublishedODataService) error { + return errUnimplemented("MovePublishedODataService") +} + func (unimplemented) MoveSnippet(_ *pages.Snippet) error { return errUnimplemented("MoveSnippet") } diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index fe3c68050..4daeaa705 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -6,6 +6,8 @@ package mprbackend import ( + "errors" + "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/types" @@ -501,6 +503,18 @@ func (b *MprBackend) UpdateDatabaseConnection(conn *model.DatabaseConnection) er func (b *MprBackend) MoveDatabaseConnection(conn *model.DatabaseConnection) error { return b.writer.MoveDatabaseConnection(conn) } +func (b *MprBackend) MoveJavaAction(ja *javaactions.JavaAction) error { + if ja == nil { + return errors.New("MoveJavaAction: nil java action") + } + return b.writer.MoveUnitByID(string(ja.ID), string(ja.ContainerID)) +} +func (b *MprBackend) MovePublishedODataService(svc *model.PublishedODataService) error { + if svc == nil { + return errors.New("MovePublishedODataService: nil service") + } + return b.writer.MoveUnitByID(string(svc.ID), string(svc.ContainerID)) +} func (b *MprBackend) DeleteDatabaseConnection(id model.ID) error { return b.writer.DeleteDatabaseConnection(id) } diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index a320725f6..a3cbfa940 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -1082,6 +1082,26 @@ func findInWidgetChildren(wDoc bson.D, widgetName string) *bsonWidgetResult { colPropKeys: colPropKeyMap, } } + // Descend into the column's OWN content widgets. A column + // rendered as customContent holds a widget tree at + // Properties[content].Value.Widgets — one level deeper than the + // pluggable search above, which only reaches the grid's own + // Object.Properties[].Value.Widgets. Without this, a widget + // inside a customContent column was unreachable by ALTER PAGE + // and the only remedy was rewriting the page (issue #834). + for _, cProp := range bsonnav.DGetArrayElements(bsonnav.DGet(colDoc, "Properties")) { + cPropDoc, ok := cProp.(bson.D) + if !ok { + continue + } + cValDoc := bsonnav.DGetDoc(cPropDoc, "Value") + if cValDoc == nil { + continue + } + if result := findInWidgetArray(cValDoc, "Widgets", widgetName); result != nil { + return result + } + } } break // only one "columns" property per widget } @@ -2352,26 +2372,32 @@ func buildDesignPropertyValueDoc(valueType, option string) bson.D { } func setWidgetCaptionMut(widget bson.D, value any) error { - caption := bsonnav.DGetDoc(widget, "Caption") - if caption == nil { - return mdlerrors.NewValidation("widget has no Caption property") + if caption := bsonnav.DGetDoc(widget, "Caption"); caption != nil { + setTranslatableText(caption, "", value) + return nil } - setTranslatableText(caption, "", value) - return nil + // An ActionButton has no `Caption` document: its caption is a + // Forms$ClientTemplate stored under `CaptionTemplate` (Template → Items[] → + // Translation.Text), the same shape setWidgetContentMut walks. Without this + // branch `alter page … set Caption = '…' on