Fix silent drops of conditional expressions with keyword-named functions

.claude/skills/fix-issue.md

@@ -364,6 +364,9 @@ cases for these three BSON types — they fell to `default: return nil`.
 | The SessionStart hook `mxcli init` writes cannot survive an idle reap: it is guarded on `test -x ./mxcli`, and `.gitignore` excludes that binary (~85 MB) on purpose. The container is reclaimed, the repo re-cloned without it, the guard fails and the hook no-ops through `\|\| true` — the next session has no mxcli, no MxBuild cache, no database, and no message saying so | The hook inlined the whole bring-up in one shell line, so "binary missing" could only be expressed as "skip". A hook line cannot reasonably do OS/arch detection and a download; something committed has to | `cmd/mxcli/init_hook.go` (`bootstrapScriptTemplate`, `writeBootstrapScript`, `sessionStartHookCommand`, `sessionStartHookMarkers`) | Emit a committed `.claude/bootstrap-mxcli.sh` that resolves OS/arch, fetches the binary when absent (`MXCLI_TAG` to pin), then runs the setup; the hook becomes `sh .claude/bootstrap-mxcli.sh \|\| true`. **Changing the hook command breaks dedupe**, which matched on the old command string — so `addSessionStartHook` now recognises *any* known marker and **rewrites the entry in place**, migrating an old project instead of leaving it with two hooks that both run. **Generalisable**: a guard whose condition is something you deliberately do not commit is a silent no-op waiting for a fresh clone — make the guard able to satisfy itself. Verified by reproducing the reap: moved `./mxcli` out of the project, ran the hook command verbatim, watched it re-download (88 MB, new mtime) and finish with "Setup complete … database ready". Tests `TestAddSessionStartHook_MigratesLegacyCommand`, `TestEnsureSessionStartHook_WritesFile`. mxcli-todo #2 |
 | `mxcli exec -p app.mpr - <<'EOF' … EOF` fails with `Error reading file: open -: no such file or directory` — `-` is taken literally as a filename, so MDL cannot be piped or written as a heredoc and every ad-hoc script needs a temp file first | `exec` (and `check`) called `os.ReadFile(path)` directly, with no case for the conventional stdin spelling | `cmd/mxcli/mdlsource.go` (new `readMDLSource`, `mdlSourceLabel`), `cmd/mxcli/cmd_exec.go`, `cmd/mxcli/cmd_check.go` | One helper both commands share, so `check` gained the same spelling rather than only the reported one; `check` reports the source as `<stdin>` instead of a bare `-`. Verified live: a heredoc through `exec` and a pipe through `check` both run. Tests `cmd/mxcli/mdlsource_test.go`. mxcli-todo #5 |
 | `mxcli syntax` documents spellings the parser rejects, so an agent following the reference writes MDL that fails — `TEXTBOX … (Binds: Attr)` ("'Binds:' is no longer supported, use 'Attribute:' instead") and `DataSource: MICROFLOW Module.MF()` (a zero-arg microflow datasource takes NO parens, unlike RETRIEVE/CALL) | Nothing checks the `syntax` corpus against the parser. `make check-skill-mdl` validates MDL blocks in the skills and the docs site, but the `Syntax`/`Example` strings in `cmd/mxcli/syntax/*.go` are not covered, so a retired spelling can sit there indefinitely | `cmd/mxcli/syntax/features_page.go` (10 × `Binds:` → `Attribute:`, the datasource parens), `cmd/mxcli/syntax/retired_spellings_test.go` (new guard) | Fix the text **and** pin it: a table-driven test fails if a retired spelling reappears in any topic's Syntax or Example. It is a spelling guard rather than a parse — the snippets are fragments (a DATAVIEW body, a property line) that do not stand alone as statements, so they cannot just be fed to the parser. Proven by reintroducing `Binds:` and watching the test name the topic and field. **A third claim in the same report did not reproduce**: `CONTAINER (OnClick: SHOW_PAGE M.P(Param: $currentObject))` parses fine on current main, so only the two verified ones were changed. mxcli-todo #8 |
+| `alter page … set Editable = [expr]` (or `set Visible`) writes a project Studio Pro refuses to open: `StorageLoadException: Conditional editability settings has an invalid value '' for property Attribute`. `mxcli check` ✓ and `mx check` ✓ — neither inspects the stored value. The identical settings written by `create page` load fine | The ALTER path builds the `Forms$Conditional{Visibility,Editability}Settings` node by hand and wrote `Attribute: null`. `Attribute` is a **BY_NAME** `AttributeIdentifier`, so its unset value is the empty string, not null — exactly what the CREATE path already encodes via `codec.RegisterTypeDefaults(..., EmptyStringFields: []string{"Attribute"})`, whose comment records this same StorageLoadException from #627. Only the hand-built ALTER node missed it | `mdl/backend/pagemutator/mutator.go` (`setWidgetConditionalSettingMut`) | Write `{Key: "Attribute", Value: ""}`, not `nil`. **General rule: when one path hand-builds BSON that another path builds through the codec, diff the two encodings rather than eyeballing the hand-built one** — `mxcli bson dump --type page --object M.P` on a CREATE-authored and an ALTER-authored widget makes the divergence a one-line diff (key sets and values were otherwise identical). `SourceVariable` stays `nil`: it is BY_ID, where null *is* the absent value, so "null is wrong" is per-field, not a blanket rule. Test `TestSetWidgetConditionalSetting_AttributeIsEmptyString`; repro `mdl-examples/bug-tests/851-alter-page-conditional-attribute.mdl`. Issue #851 |
+| A widget conditional using a function whose name is also an MDL lexer keyword — `visible: [trim($currentObject/Slug) != '']`, `[length(…) > 0]`, `empty`/`count`/`find` — **silently drops the whole property**; `mxcli check` ✓, `mx check` ✓, and the widget renders unconditionally visible. `toUpperCase`/`isMatch`/`contains` in the same position work | `xpathFunctionName` (MDLPage.g4) enumerated only `IDENTIFIER \| HYPHENATED_ID \| NOT \| TRUE \| FALSE \| CONTAINS`, so `trim(` never matched `xpathFunctionCall`. The enclosing `[...]` then failed to parse as an `xpathConstraint` and matched the generic `propertyValueV3` alternative instead, so the visitor set `Visible` (an array) rather than `VisibleIf`, and the builder's `else if pages.StaticVisibleExpression(...)` — which reads only bool/string — never fired | `mdl/grammar/domains/MDLPage.g4` (`xpathFunctionName`) + `mdl/executor/validate_widgets.go` (`validateConsumableConditional`) | Define `xpathFunctionName : xpathWord \| NOT` — `xpathWord` is a negated token set, so it self-maintains as the lexer gains keywords; an enumerated list reacquires this bug with the next promoted function name. Safe because `xpathFunctionCall` requires a following LPAREN and no `xpathStepValue` may be followed by one, so bare `empty` still parses as a path word. `NOT` is spelled out (xpathWord excludes it). **Also add the general guard**: MDL-WIDGET19 errors when `Visible`/`Editable` holds a value that is neither routed to `VisibleIf`/`EditableIf` nor a bool/string — that is the residue signature of any conditional the visitor could not build, so the next one fails loudly instead of vanishing. `make grammar` regenerates the parser (not committed). **Verify in a browser, not at `mx check`** — a dropped property is still a valid model, so `mx check` reports 0 errors before AND after; the symptom only exists at render time (see `verify-in-runtime.md`). The repro script carries a `Bug852.Verify` page for this: `Slug` is three spaces, so `trim()` changes the outcome and a dropped `Visible` renders (Mendix defaults to visible). Pre-fix all 5 markers render; post-fix only the 3 that should. **One rule, two contexts**: `xpathConstraint` serves both `Visible:`/`Editable:` (a Mendix *client expression* — trim/length/toUpperCase/find) and a datasource `where` (real *XPath* — contains/starts-with/ends-with/string-length/not, `length()` = list length, aggregates Java-only, and `empty`/`NULL` are KEYWORDS not calls). The sets differ, so the grammar must not enumerate either; mxbuild adjudicates. Regression-test the XPath side when touching this rule — `[Name = empty]`, `[Name = NULL]`, `not()`, `contains()`, `starts-with()`, `string-length()` all still parse and `mx check` clean. Tests `TestConditionalVisibility_KeywordFunctionNames`, `TestValidateStaticWidget_UnconsumableConditional`; repro `mdl-examples/bug-tests/852-conditional-keyword-functions.mdl`. Issue #852 |
+| `download file $Doc;` is accepted by `mxcli check` and `mxcli exec` ("Created microflow") but the activity lands with **no action at all** — `describe` renders `-- Empty action` and `mx check` fails `[CE0008] "No action defined."`. Same for `download file $Doc show in browser;` | `microflowActionToGen` (the modelsdk write path) had no `*microflows.DownloadFileAction` case, so it hit `default: return nil` and the enclosing ActionActivity was serialized with a nil Action. Grammar, visitor, flow builder, read path and DESCRIBE formatter were all already in place, so the statement passed every stage that reports anything and vanished at the one that does not | `mdl/backend/modelsdk/microflow_write.go` (`microflowActionToGen`) | Add the case, setting `FileDocumentVariableName`, `ShowFileInBrowser` and `ErrorHandlingType` (Rollback default). **The storage key is `ShowFileInBrowser`, not `ShowInBrowser`** — the gen setter binds the right one; legacy's `parseDownloadFileAction` reads the wrong key. **Test at the round trip, not the reader**: a reader-only test starts from BSON the writer never had to produce, so `TestActionFromGen_DownloadFile` was green throughout. `roundTripMicroflow` (model→gen→codec→model) is the harness; assert the ActionActivity's `Action` is non-nil, which is the CE0008 shape itself. This is the same silent-drop mechanism as the `microflowObjectToGen` default branch (#791) — when auditing, diff the write switch's cases against `sdk/mpr/writer_microflow_actions.go`. Test `TestMicroflowRoundTrip_DownloadFile`; repro `mdl-examples/bug-tests/850-download-file-action.mdl`. Issue #850 |
 | A decision written with **uppercase** keywords — `IF $T/Status != M.Status.Done AND $T/CompletedOn != empty` — passes `mxcli check` and then fails the build with `[error] [CE0117] "Error(s) in expression."`, quoting the expression back with `AND` still uppercase. The same condition written with `=` builds fine, which makes it look like `!=` cannot be an operand of `AND` | Mendix requires its word operators lowercase. A rebuilt `BinaryExpr` gets `strings.ToLower(e.Operator)`; a condition kept as an `ast.SourceExpr` (original text **plus** the parsed tree) returned `e.Source` verbatim and skipped it. The `=` form parses to a BinaryExpr and the `!=` form to a SourceExpr — hence the operator-shaped illusion | `mdl/executor/cmd_microflows_helpers.go` (`normalizeMendixOperatorCase`, applied in the `SourceExpr` branch) | Lowercase `and/or/not/div/mod` in preserved source, leaving everything else byte-identical: a scanner that tracks single-quoted literals (with `''` escapes) and skips any word preceded by `.`, `/` or `$`, so `'AND'`, `M.Enum.And`, `$Task/Mod` and `$Android` are untouched. **The reporter's own probe table is the cautionary bit** — nine builds established a rule ("any `!=` inside `AND` fails") that was real in every observation and wrong about the cause, because every failing probe was uppercase and the control was not. When a table's rule tracks a token, check what ELSE differs between the rows. Reproduced and fixed against mxbuild 11.12.1: stored `AND` → CE0117, stored `and` → 0 errors. Repro `mdl-examples/bug-tests/todo-14b-uppercase-and-operator.mdl`; tests `TestNormalizeMendixOperatorCase`. mxcli-todo #14b |
 | `ALTER ENTITY X ADD EVENT HANDLER …` errors when the handler exists and `DROP EVENT HANDLER …` errors when it does not, so a script containing either cannot be re-run — and a defensive drop-then-add fails on whichever half does not match. `ADD ATTRIBUTE` has `IF NOT EXISTS`; event handlers had no equivalent | The idempotency guards added for attributes (findings #10) were never extended to the event-handler clauses, which are the one member with no other re-run route | `mdl/grammar/domains/MDLDomainModel.g4` (`ifNotExists?` / `ifExists?` on the event-handler clauses), `mdl/visitor/visitor_entity.go`, `mdl/executor/cmd_entities.go` | Reuse the existing `ifNotExists`/`ifExists` grammar rules rather than inventing a second spelling, so the guard reads the same everywhere. The error messages now name the flag, so the fix is discoverable from the failure. Verified by running the same script twice: first run drops (skipping, absent) then adds; second run skips both, exit 0, and the project still builds 0 errors. mxcli-todo #18 |
 | `CREATE DEMO USER` reports success and `SHOW PROJECT SECURITY` reports `Demo Users Enabled: true`, yet the running app has **zero** accounts — `SELECT Name FROM Administration.Account` returns 0 rows, there is no login page, and none of the row-level XPath rules are enforced | A blank mxcli template ships with **Security Level: Off**, and with it off the runtime creates no accounts at all. The demo users are written to the model correctly; nothing connected the two facts, so the model said yes and the app said nothing | `mdl/executor/cmd_security_write.go` (`warnDemoUsersInert`, called after a successful create) | Say it at the moment the user would otherwise believe it worked, and name the one statement that fixes it (`alter project security level prototype`). A *warning*, not a refusal: authoring demo users before raising the level is legitimate ordering. **Generalisable**: when a write succeeds but a project-level setting makes it inert, the write path is the only place with both facts in hand. Tests `TestWarnDemoUsersInert`. mxcli-todo #15 |

.claude/skills/mendix/create-page.md

@@ -1082,8 +1082,38 @@ textbox txtHidden (label: 'Hidden', attribute: Name, visible: false)
 -- A quoted-string expression is also accepted (CREATE and ALTER). Unlike the
 -- bracket form, it is NOT auto-rooted — write $currentObject/ yourself.
 dynamictext ovChip (content: 'chip', visible: '$currentObject/Name != empty')
+
+-- Function calls work in the bracket form, including functions whose name is
+-- also an MDL keyword (trim, length, find). Arguments are rooted like any other
+-- reference.
+dynamictext tTrim (content: 'x', visible: [trim($currentObject/Slug) != ''])
+textbox txtSlug (label: 'Slug', attribute: Slug, editable: [length(Slug) > 0])
 ```
 
+> **`visible:`/`editable:` is a Mendix *expression*, not XPath** — a different
+> function set from a datasource `where` clause, even though both use `[ ... ]`:
+>
+> | | `visible:` / `editable:` (client expression) | `where [ … ]` (XPath) |
+> |---|---|---|
+> | String tests | `trim()`, `length()`, `toUpperCase()`, `find()`, `contains()` | `contains()`, `starts-with()`, `ends-with()`, `string-length()` |
+> | `length()` | character count | number of elements in a list |
+> | Emptiness | `$currentObject/X != ''` / `!= empty` | `[X = empty]` or `[X = NULL]` — a **keyword**, never `empty(…)` |
+> | Aggregates | not available | `count()`/`avg()`/`min()`/`max()`/`sum()` are Java-API-only |
+>
+> mxcli's grammar accepts any function name in both and lets MxBuild adjudicate,
+> so a wrong-context call surfaces as **CE0117** "Error(s) in expression" at
+> build rather than as a parse error. See the Mendix reference guide:
+> [XPath constraint functions](https://docs.mendix.com/refguide/xpath-constraint-functions/),
+> [XPath keywords](https://docs.mendix.com/refguide/xpath-keywords-and-system-variables/).
+
+> **An unparseable conditional is an error, not a silent drop.** If the
+> expression inside `visible: [ ... ]` / `editable: [ ... ]` can't be parsed, the
+> property has nowhere to go and would vanish on write — leaving the widget
+> unconditionally visible/editable, which looks identical to a specificity bug in
+> the running app. `mxcli check` reports this as **MDL-WIDGET19** and fails the
+> command instead. Until v0.16.x, `trim(…)` and `length(…)` hit exactly this path
+> and disappeared without a word (issue #852).
+
 > **Attribute rooting is automatic** — a bare attribute in a widget
 > visibility/editability expression (`[Name != '']`, `[IsActive]`) is rooted in the
 > widget data context as `$currentObject/Name != ''` for you, so it no longer

mdl-examples/bug-tests/850-download-file-action.mdl

@@ -0,0 +1,49 @@
+-- ============================================================================
+-- Issue #850 — `download file` parsed but never written to the project
+-- ============================================================================
+--
+-- `microflowActionToGen` (mdl/backend/modelsdk/microflow_write.go) switches on
+-- the semantic action type and had no `*microflows.DownloadFileAction` case, so
+-- the action fell through to `default: return nil` and the enclosing
+-- ActionActivity was written with no Action at all.
+--
+-- Everything either side of the writer was already in place — grammar, visitor,
+-- flow builder, read path and DESCRIBE formatter — so the statement was accepted
+-- at every stage that reports anything and vanished at the one stage that does
+-- not. `mxcli check` passed, `mxcli exec` printed "Created microflow", and only
+-- `mx check` noticed:
+--
+--   [error] [CE0008] "No action defined." at Action activity 'Activity'
+--
+-- To verify:
+--   1. Run this script.
+--   2. `mxcli describe microflow Bug850.ACT_Download` — the body must render
+--      `download file $Doc;`, NOT `-- Empty action`. Likewise
+--      `Bug850.ACT_DownloadInBrowser` must render `… show in browser;`.
+--   3. mx check should report 0 errors (CE0008 before the fix).
+--
+-- Note the storage key is ShowFileInBrowser, not ShowInBrowser.
+-- ============================================================================
+
+CREATE MODULE Bug850;
+
+CREATE OR MODIFY PERSISTENT ENTITY Bug850.Doc EXTENDS System.FileDocument (
+);
+
+CREATE OR MODIFY MICROFLOW Bug850.ACT_Download (
+  $Doc: Bug850.Doc
+)
+RETURNS Boolean
+BEGIN
+  download file $Doc;
+  return true;
+END;
+
+CREATE OR MODIFY MICROFLOW Bug850.ACT_DownloadInBrowser (
+  $Doc: Bug850.Doc
+)
+RETURNS Boolean
+BEGIN
+  download file $Doc show in browser;
+  return true;
+END;