From bf724ac7e11c9c9bf91ca3d0a2b1acbe20b0b89e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 14:22:05 +0000 Subject: [PATCH 01/21] Add repeatable upstream-PR link generator (script + /mxcli-dev:upstream-pr) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generating a cross-fork compare URL to merge ako/mxcli:main into mendixlabs/mxcli:main was a recurring manual task — mendixlabs/mxcli is not in tooling scope, so the PR can't be opened via API and we hand the user a prefilled compare link instead. - scripts/upstream-pr-link.sh: URL-encodes a title + Markdown body into a ?title=&body= compare URL. Defaults to ako/mxcli:main -> mendixlabs:main; --commits auto-builds the body from a git range; --body-file reads a hand-written body (stdin via -). Handles newlines/backticks/ampersands. - .claude/commands/mxcli-dev/upstream-pr.md: contributor slash command that wraps the script (draft title/body, generate link, present plain-text fallback, note the out-of-scope caveat). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/commands/mxcli-dev/upstream-pr.md | 46 +++++++++ scripts/upstream-pr-link.sh | 111 ++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 .claude/commands/mxcli-dev/upstream-pr.md create mode 100755 scripts/upstream-pr-link.sh diff --git a/.claude/commands/mxcli-dev/upstream-pr.md b/.claude/commands/mxcli-dev/upstream-pr.md new file mode 100644 index 000000000..80fd56e08 --- /dev/null +++ b/.claude/commands/mxcli-dev/upstream-pr.md @@ -0,0 +1,46 @@ +# /mxcli-dev:upstream-pr — Link to open a PR into upstream (mendixlabs/mxcli) + +Generate a prefilled GitHub **compare** URL that opens a PR merging this fork +(`ako/mxcli`) into the upstream fork (`mendixlabs/mxcli`). + +**Why a link instead of opening the PR directly:** `mendixlabs/mxcli` is not in +this session's tooling scope, so the PR can't be created via the GitHub API. The +compare URL prefills the title and body; the user opens it and clicks "Create +pull request". + +## Steps + +1. Confirm what's actually unmerged upstream. If you have (or can fetch) the + upstream base, build the range explicitly: + ```bash + git fetch https://github.com/mendixlabs/mxcli main + git log --no-merges --oneline FETCH_HEAD..HEAD + ``` + If the fetch is blocked or unnecessary, fall back to summarising the fork's + `main` since the last sync. +2. Draft a concise **title** and a Markdown **body** grouping the changes by + theme (one bullet per finding/fix). Reuse the structure from the last sync PR. +3. Generate the link with the script — pass the body on stdin so multi-line + Markdown encodes cleanly: + ```bash + scripts/upstream-pr-link.sh --title "" --body-file - <<'BODY' + <markdown body> + BODY + ``` + Or let it auto-build the body from commits: + ```bash + scripts/upstream-pr-link.sh --commits FETCH_HEAD..HEAD + ``` +4. Present to the user: + - the prefilled compare URL, + - the **title** and **body** as plain text (fallback if the browser trims a + long prefilled body). +5. Remind the user that `mendixlabs/mxcli` isn't in scope, so this is a link — + offer to `add_repo` and open the PR via API if they'd rather. + +## Notes + +- Defaults are `ako/mxcli:main → mendixlabs/mxcli:main`. Override with + `--fork`, `--upstream`, `--base`, `--head` for other syncs. +- Do **not** include the model identifier in the title or body. +- This is a link generator only — it does not push, commit, or open anything. diff --git a/scripts/upstream-pr-link.sh b/scripts/upstream-pr-link.sh new file mode 100755 index 000000000..0633f1ddb --- /dev/null +++ b/scripts/upstream-pr-link.sh @@ -0,0 +1,111 @@ +#!/bin/bash + +# upstream-pr-link.sh — print a prefilled GitHub compare URL for opening a PR +# that merges this fork's branch into an upstream fork's branch. +# +# Why: mendixlabs/mxcli (the upstream) is not in tooling scope, so we can't open +# the PR via API. Instead we generate a cross-fork *compare* URL with the title +# and body prefilled, which the user opens in a browser and clicks "Create". +# +# The mechanical part — URL-encoding a multi-line title + Markdown body into the +# `?title=…&body=…` query — is what this script automates. +# +# Usage: +# scripts/upstream-pr-link.sh \ +# [--upstream mendixlabs/mxcli] [--fork ako/mxcli] \ +# [--base main] [--head main] \ +# [--title "…"] [--body-file path] \ +# [--commits <git-range>] +# +# Defaults reproduce the common case: merge ako/mxcli:main → mendixlabs/mxcli:main. +# +# --title PR title. Default: "Sync <fork>: <head> → <upstream>:<base>". +# --body-file File whose contents become the PR body (Markdown). Use "-" for stdin. +# --commits A git revision range (e.g. origin/upstream-main..HEAD). When given +# and no --body-file is set, the body is auto-built from the +# one-line commit log over that range. +# +# Examples: +# # simplest — just the link with default title and a one-line body: +# scripts/upstream-pr-link.sh +# +# # supply a hand-written body: +# scripts/upstream-pr-link.sh --body-file /tmp/pr-body.md +# +# # auto-build the body from commits not yet upstream: +# git fetch https://github.com/mendixlabs/mxcli main +# scripts/upstream-pr-link.sh --commits FETCH_HEAD..HEAD + +set -euo pipefail + +UPSTREAM="mendixlabs/mxcli" +FORK="ako/mxcli" +BASE="main" +HEAD="main" +TITLE="" +BODY_FILE="" +COMMITS="" + +usage() { + sed -n '3,40p' "$0" | sed 's/^# \{0,1\}//' + exit "${1:-2}" +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --upstream) UPSTREAM="${2:?--upstream requires owner/repo}"; shift 2 ;; + --fork) FORK="${2:?--fork requires owner/repo}"; shift 2 ;; + --base) BASE="${2:?--base requires a branch}"; shift 2 ;; + --head) HEAD="${2:?--head requires a branch}"; shift 2 ;; + --title) TITLE="${2:?--title requires text}"; shift 2 ;; + --body-file) BODY_FILE="${2:?--body-file requires a path}"; shift 2 ;; + --commits) COMMITS="${2:?--commits requires a git range}"; shift 2 ;; + -h|--help) usage 0 ;; + *) echo "unknown argument: $1" >&2; usage 2 ;; + esac +done + +# owner:repo (fork owner) form for the cross-fork head ref in a compare URL. +FORK_OWNER="${FORK%%/*}" +FORK_REPO="${FORK##*/}" + +if [[ -z "$TITLE" ]]; then + TITLE="Sync ${FORK}: ${HEAD} → ${UPSTREAM}:${BASE}" +fi + +# Resolve the body text. +BODY="" +if [[ -n "$BODY_FILE" ]]; then + if [[ "$BODY_FILE" == "-" ]]; then + BODY="$(cat)" + else + BODY="$(cat "$BODY_FILE")" + fi +elif [[ -n "$COMMITS" ]]; then + BODY="Merges \`${FORK}:${HEAD}\` into \`${UPSTREAM}:${BASE}\`."$'\n\n'"### Commits"$'\n' + BODY+="$(git log --no-merges --pretty='- %s' "$COMMITS")" +else + BODY="Merges \`${FORK}:${HEAD}\` into \`${UPSTREAM}:${BASE}\`." +fi + +# URL-encode title/body and assemble the compare URL. Python3 handles RFC-3986 +# percent-encoding of newlines, backticks, and Markdown reliably. +TITLE="$TITLE" BODY="$BODY" \ +UPSTREAM="$UPSTREAM" BASE="$BASE" FORK_OWNER="$FORK_OWNER" FORK_REPO="$FORK_REPO" HEAD="$HEAD" \ +python3 - <<'PY' +import os, urllib.parse + +upstream = os.environ["UPSTREAM"] +base = os.environ["BASE"] +owner = os.environ["FORK_OWNER"] +repo = os.environ["FORK_REPO"] +head = os.environ["HEAD"] + +q = urllib.parse.urlencode({ + "expand": "1", + "title": os.environ["TITLE"], + "body": os.environ["BODY"], +}) + +print(f"https://github.com/{upstream}/compare/{base}...{owner}:{repo}:{head}?{q}") +PY From 253d60d8abfd09089b87bb402338e94b8f35c424 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:30:52 +0000 Subject: [PATCH 02/21] fix(workflows): version-gate call-microflow storage name at Mendix 11.9 (FINDINGS #39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow whose flow contains a "call microflow" activity was written with the on-disk $Type `Workflows$CallMicroflowTask`. Both checkers passed (mxcli check ✓, mx check → 0 errors), but on an 11.9+ project the runtime refused to load the ENTIRE model at boot: Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the on-disk $Type. Evidence from the cached toolchains: the 11.6.3 modeler knows only CallMicroflowTask; the 11.10 modeler carries both (the old one marked "Removed due to code refactoring ... WOR-2802") plus a conversion routine; the 11.10+ runtime metamodel jars know only CallMicroflowActivity. The 11.9 boundary matches the existing HasOwner→HasOwnerAttr domain-model gate. Fix: emit CallMicroflowActivity for projects >= 11.9 and keep CallMicroflowTask for older ones. The semantic model is unchanged — only the emitted $Type differs — so the tree is built with the legacy name and rewritten when targeting 11.9+. - modelsdk (default engine): applyCallMicroflowStorageName walks the built element tree; useCallMicroflowActivityName() gates on pv.IsAtLeast(11,9). Codec TypeDefaults + list-marker registered under both $Type names. Wired into CreateWorkflow/UpdateWorkflow and the ALTER-workflow activity serializer. - legacy engine: renameCallMicroflowTypeBSON rewrites the serialized BSON, gated the same way in serializeWorkflow and SerializeWorkflowActivity. - read path already folds both gen types into the one semantic CallMicroflowTask. Tests: rename walk (both directions) + encode-validity under the new name; version gate against the vendored 11.6.6 fixture; legacy BSON-rewrite unit test. Repro mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl; symptom row added to fix-issue.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + ...63-workflow-callmicroflow-storage-name.mdl | 44 +++++++++ .../modelsdk/workflow_mutator_write.go | 7 +- .../modelsdk/workflow_storagename_test.go | 91 +++++++++++++++++++ mdl/backend/modelsdk/workflow_write.go | 59 ++++++++++-- mdl/backend/mpr/backend.go | 10 +- mdl/backend/mpr/workflow_mutator.go | 2 +- sdk/mpr/workflow_write_test.go | 36 ++++++++ sdk/mpr/writer_workflow.go | 40 +++++++- 9 files changed, 276 insertions(+), 14 deletions(-) create mode 100644 mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl create mode 100644 mdl/backend/modelsdk/workflow_storagename_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 786e7f5b9..d8e02733d 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -23,6 +23,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | `describe` shows `$var = list operation %T;` (with type name) | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatListOperation()` | Add `case *microflows.XxxOperation:` before the `default` | | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | +| Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | | `grant view on page` / `grant execute on microflow\|nanoflow` / `grant access on odata\|published rest service` to a role from **another module** passes `mxcli check`/`exec` but fails the Mendix build with **CE0148 "reselect roles"** — the own-module role works | Document access (page/microflow/nanoflow/service `AllowedModuleRoles`) may only reference the document's **own** module roles; Studio Pro's picker only offers those. The grant path wrote `role.Module + "." + role.Name` verbatim with no same-module check (only `validateModuleRole` = role-exists-in-its-module), so a cross-module reference reached the model. The MOVE path already guarded this (`remapDocumentAccessRoles`) — GRANT didn't | `mdl/executor/cmd_security_defaults.go` (`checkDocumentAccessRolesSameModule`) + the 5 grant handlers in `mdl/executor/cmd_security_write.go` | Pre-check each grant: reject when any `role.Module != docModule` with an actionable message (name the doc's module + suggest the own-module role). Reject (don't silently remap) — a GRANT is explicit, so a wrong role/doc shouldn't be substituted. Wired into page/microflow/nanoflow/OData/published-REST grants. Repro `mdl-examples/bug-tests/ce0148-cross-module-grant.mdl` | | CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` | diff --git a/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl b/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl new file mode 100644 index 000000000..1ec7909b6 --- /dev/null +++ b/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl @@ -0,0 +1,44 @@ +-- ============================================================================ +-- FINDINGS #39: workflow "call microflow" activity storage name (Mendix 11.9+) +-- ============================================================================ +-- +-- Symptom: a workflow whose flow contains a "call microflow" activity was written +-- with the on-disk $Type `Workflows$CallMicroflowTask`. Both checkers passed +-- (`mxcli check` ✓, `mx check` → 0 errors), but on an 11.9+ project the runtime +-- refused to load the ENTIRE model at boot: +-- +-- Failed to load model: ... No new model classes have arrived within ten +-- seconds, aborting model initialization +-- (Class 'Workflows$CallMicroflowTask' could not be found). +-- +-- Root cause: Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into +-- CallMicroflowActivity + AIAgentTaskActivity, renaming the on-disk $Type from the +-- pre-11.9 CallMicroflowTask to CallMicroflowActivity. Evidence: the 11.6.3 modeler +-- knows only CallMicroflowTask; the 11.10 modeler carries both (the old one marked +-- "Removed due to code refactoring ... WOR-2802") plus a conversion routine; the +-- 11.10+ runtime metamodel jars know only CallMicroflowActivity. +-- +-- After fix: mxcli emits CallMicroflowActivity for projects >= 11.9 and keeps +-- CallMicroflowTask for older projects (version-gated, mirroring the existing +-- HasOwner→HasOwnerAttr 11.9 gate). The semantic model is unchanged; only the +-- emitted $Type differs. +-- +-- Verify (on an 11.9+ project, e.g. Mendix 11.12.1): +-- mxcli exec 263-workflow-callmicroflow-storage-name.mdl -p App.mpr +-- <mxbuild>/modeler/mx check App.mpr -> 0 errors +-- mxcli run --local -p App.mpr --ensure-db -> model loads (previously died) +-- ============================================================================ + +create or modify microflow "MyFirstModule"."ACT_ApproveStep" () +begin + return; +end; +/ + +create or modify workflow "MyFirstModule"."WF_CallMicroflowStorageName" + parameter $Context: MyFirstModule.Ctx +begin + call microflow "MyFirstModule"."ACT_ApproveStep" + comment 'automated approve step'; +end workflow; +/ diff --git a/mdl/backend/modelsdk/workflow_mutator_write.go b/mdl/backend/modelsdk/workflow_mutator_write.go index 39181a48d..94863d3af 100644 --- a/mdl/backend/modelsdk/workflow_mutator_write.go +++ b/mdl/backend/modelsdk/workflow_mutator_write.go @@ -36,7 +36,7 @@ func (b *Backend) OpenWorkflowForMutation(unitID model.ID) (backend.WorkflowMuta // SerializeWorkflowActivity converts a domain WorkflowActivity to its raw bson.D // form via the codec converters (used by the ALTER WORKFLOW insert/replace paths). func (b *Backend) SerializeWorkflowActivity(a workflows.WorkflowActivity) (any, error) { - d := serializeWorkflowActivityToBSON(a) + d := serializeWorkflowActivityToBSON(a, b.useCallMicroflowActivityName()) if d == nil { return nil, fmt.Errorf("SerializeWorkflowActivity: unsupported activity %T", a) } @@ -49,7 +49,7 @@ type codecWorkflowDeps struct{ b *Backend } var _ wfmutator.Deps = codecWorkflowDeps{} func (d codecWorkflowDeps) SerializeWorkflowActivity(a workflows.WorkflowActivity) bson.D { - return serializeWorkflowActivityToBSON(a) + return serializeWorkflowActivityToBSON(a, d.b.useCallMicroflowActivityName()) } func (d codecWorkflowDeps) SaveUnit(unitID string, contents []byte) error { @@ -60,11 +60,12 @@ func (d codecWorkflowDeps) SaveUnit(unitID string, contents []byte) error { // codec (activityToGen → Encode) and decodes the result to bson.D, so the shared // wfmutator can splice it into the raw workflow tree. Returns nil for unsupported // activity types. -func serializeWorkflowActivityToBSON(a workflows.WorkflowActivity) bson.D { +func serializeWorkflowActivityToBSON(a workflows.WorkflowActivity, useActivityName bool) bson.D { el := activityToGen(a) if el == nil { return nil } + applyCallMicroflowStorageName(el, useActivityName) raw, err := (&codec.Encoder{}).Encode(el) if err != nil { return nil diff --git a/mdl/backend/modelsdk/workflow_storagename_test.go b/mdl/backend/modelsdk/workflow_storagename_test.go new file mode 100644 index 000000000..055c79e2d --- /dev/null +++ b/mdl/backend/modelsdk/workflow_storagename_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/modelsdk/element" + "github.com/mendixlabs/mxcli/sdk/workflows" +) + +// buildCallMicroflowWorkflowGen builds the gen element tree for a workflow whose +// flow contains a single "call microflow" activity. +func buildCallMicroflowWorkflowGen() element.Element { + wf := &workflows.Workflow{ + Name: "SnFlow", + WorkflowName: "Sn Flow", + Parameter: &workflows.WorkflowParameter{EntityRef: "MyFirstModule.Ctx"}, + Flow: &workflows.Flow{ + Activities: []workflows.WorkflowActivity{ + &workflows.StartWorkflowActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "Start"}}, + &workflows.CallMicroflowTask{ + BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "Call", Caption: "Call"}, + Microflow: "MyFirstModule.ACT_Do", + }, + &workflows.EndWorkflowActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "End"}}, + }, + }, + } + return workflowToGen(wf) +} + +func countTypeName(root element.Element, typ string) int { + n := 0 + element.Walk(root, func(e element.Element) bool { + if e.TypeName() == typ { + n++ + } + return true + }) + return n +} + +// TestApplyCallMicroflowStorageName verifies the version-gated $Type rewrite +// (FINDINGS #39): pre-11.9 keeps CallMicroflowTask; 11.9+ rewrites to +// CallMicroflowActivity. Writing the wrong name makes the runtime fail to load +// the whole model, and both mxcli check and mx check pass regardless — so this is +// the only guard. +func TestApplyCallMicroflowStorageName(t *testing.T) { + // useActivity=false (pre-11.9): tree keeps the legacy CallMicroflowTask name. + g := buildCallMicroflowWorkflowGen() + applyCallMicroflowStorageName(g, false) + if got := countTypeName(g, callMicroflowTaskType); got != 1 { + t.Errorf("pre-11.9: CallMicroflowTask count = %d, want 1", got) + } + if got := countTypeName(g, callMicroflowActivityType); got != 0 { + t.Errorf("pre-11.9: CallMicroflowActivity count = %d, want 0", got) + } + + // useActivity=true (11.9+): the activity is rewritten to CallMicroflowActivity. + g2 := buildCallMicroflowWorkflowGen() + applyCallMicroflowStorageName(g2, true) + if got := countTypeName(g2, callMicroflowActivityType); got != 1 { + t.Errorf("11.9+: CallMicroflowActivity count = %d, want 1", got) + } + if got := countTypeName(g2, callMicroflowTaskType); got != 0 { + t.Errorf("11.9+: CallMicroflowTask count = %d, want 0", got) + } + + // The 11.9+ tree must still encode cleanly — the codec looks up TypeDefaults + // and list markers by $Type, so both must be registered under the new name. + if _, err := (&codec.Encoder{}).Encode(g2); err != nil { + t.Fatalf("encode with CallMicroflowActivity name: %v", err) + } +} + +// TestUseCallMicroflowActivityName_Pre119 checks the version gate against the +// vendored fixture (Mendix 11.6.6, i.e. < 11.9): it must select the legacy name. +func TestUseCallMicroflowActivityName_Pre119(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + if b.useCallMicroflowActivityName() { + t.Errorf("fixture is Mendix 11.6.6 (< 11.9); useCallMicroflowActivityName() = true, want false") + } +} diff --git a/mdl/backend/modelsdk/workflow_write.go b/mdl/backend/modelsdk/workflow_write.go index ed43dfc7c..a837ce7c5 100644 --- a/mdl/backend/modelsdk/workflow_write.go +++ b/mdl/backend/modelsdk/workflow_write.go @@ -18,7 +18,8 @@ func init() { // for every activity and outcome $Type that can lead such a list. for _, t := range []string{ "Workflows$SingleUserTaskActivity", "Workflows$MultiUserTaskActivity", - "Workflows$CallMicroflowTask", "Workflows$CallWorkflowActivity", + "Workflows$CallMicroflowTask", "Workflows$CallMicroflowActivity", + "Workflows$CallWorkflowActivity", "Workflows$ExclusiveSplitActivity", "Workflows$ParallelSplitActivity", "Workflows$JumpToActivity", "Workflows$WaitForTimerActivity", "Workflows$WaitForNotificationActivity", "Workflows$StartWorkflowActivity", @@ -53,10 +54,14 @@ func init() { NullFields: []string{"Annotation"}, }) } - codec.RegisterTypeDefaults("Workflows$CallMicroflowTask", codec.TypeDefaults{ - MandatoryListMarkers: map[string]int32{"Outcomes": 3, "BoundaryEvents": 2, "ParameterMappings": 2}, - NullFields: []string{"Annotation"}, - }) + // Both the pre-11.9 CallMicroflowTask and the 11.9+ CallMicroflowActivity + // storage names share the same shape (see applyCallMicroflowStorageName). + for _, t := range []string{"Workflows$CallMicroflowTask", "Workflows$CallMicroflowActivity"} { + codec.RegisterTypeDefaults(t, codec.TypeDefaults{ + MandatoryListMarkers: map[string]int32{"Outcomes": 3, "BoundaryEvents": 2, "ParameterMappings": 2}, + NullFields: []string{"Annotation"}, + }) + } codec.RegisterTypeDefaults("Workflows$CallWorkflowActivity", codec.TypeDefaults{ MandatoryListMarkers: map[string]int32{"BoundaryEvents": 2, "ParameterMappings": 2}, NullFields: []string{"Annotation"}, @@ -87,6 +92,42 @@ func init() { }) } +// Workflow "call microflow" activity storage names. Mendix 11.9 (WOR-2802) split +// MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming +// the on-disk $Type from the older CallMicroflowTask. Writing the pre-11.9 name to +// an 11.9+ project makes the runtime fail to load the *entire* model with +// "Class 'Workflows$CallMicroflowTask' could not be found" — both checkers pass, so +// the failure only surfaces at boot (FINDINGS #39). The semantic model uses one +// activity type; only the emitted $Type differs, so we build with the legacy name +// and rewrite the tree here when targeting 11.9+. +const ( + callMicroflowTaskType = "Workflows$CallMicroflowTask" + callMicroflowActivityType = "Workflows$CallMicroflowActivity" +) + +// useCallMicroflowActivityName reports whether the target project is Mendix 11.9+ +// and therefore expects the CallMicroflowActivity storage name. +func (b *Backend) useCallMicroflowActivityName() bool { + pv := b.ProjectVersion() + return pv != nil && pv.IsAtLeast(11, 9) +} + +// applyCallMicroflowStorageName rewrites every CallMicroflowTask $Type in the tree +// to the 11.9+ CallMicroflowActivity name when useActivity is set. No-op otherwise. +func applyCallMicroflowStorageName(root element.Element, useActivity bool) { + if !useActivity || root == nil { + return + } + element.Walk(root, func(e element.Element) bool { + if e.TypeName() == callMicroflowTaskType { + if s, ok := e.(interface{ SetTypeName(string) }); ok { + s.SetTypeName(callMicroflowActivityType) + } + } + return true + }) +} + // CreateWorkflow inserts a new Workflows$Workflow document. Mirrors the legacy // serializer field-for-field via direct-build helpers. func (b *Backend) CreateWorkflow(wf *workflows.Workflow) error { @@ -100,7 +141,9 @@ func (b *Backend) CreateWorkflow(wf *workflows.Workflow) error { wf.ID = model.ID(mmpr.GenerateID()) } wf.TypeName = "Workflows$Workflow" - contents, err := (&codec.Encoder{}).Encode(workflowToGen(wf)) + g := workflowToGen(wf) + applyCallMicroflowStorageName(g, b.useCallMicroflowActivityName()) + contents, err := (&codec.Encoder{}).Encode(g) if err != nil { return fmt.Errorf("CreateWorkflow: encode: %w", err) } @@ -116,7 +159,9 @@ func (b *Backend) UpdateWorkflow(wf *workflows.Workflow) error { return fmt.Errorf("UpdateWorkflow: not connected for writing") } wf.TypeName = "Workflows$Workflow" - contents, err := (&codec.Encoder{}).Encode(workflowToGen(wf)) + g := workflowToGen(wf) + applyCallMicroflowStorageName(g, b.useCallMicroflowActivityName()) + contents, err := (&codec.Encoder{}).Encode(g) if err != nil { return fmt.Errorf("UpdateWorkflow: encode: %w", err) } diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index 73410a4d7..fe3c68050 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -846,6 +846,14 @@ func (b *MprBackend) SerializeDataSource(ds pages.DataSource) (any, error) { return mpr.SerializeCustomWidgetDataSource(ds), nil } +// useCallMicroflowActivityName reports whether the target project is Mendix 11.9+ +// and therefore expects the CallMicroflowActivity workflow storage name (see +// sdk/mpr.renameCallMicroflowTypeBSON and FINDINGS #39). +func (b *MprBackend) useCallMicroflowActivityName() bool { + pv := b.ProjectVersion() + return pv != nil && pv.IsAtLeast(11, 9) +} + func (b *MprBackend) SerializeWorkflowActivity(a workflows.WorkflowActivity) (any, error) { - return mpr.SerializeWorkflowActivity(a), nil + return mpr.SerializeWorkflowActivity(a, b.useCallMicroflowActivityName()), nil } diff --git a/mdl/backend/mpr/workflow_mutator.go b/mdl/backend/mpr/workflow_mutator.go index 59c288194..7bac8489a 100644 --- a/mdl/backend/mpr/workflow_mutator.go +++ b/mdl/backend/mpr/workflow_mutator.go @@ -38,7 +38,7 @@ type mprWorkflowDeps struct{ backend *MprBackend } var _ wfmutator.Deps = (*mprWorkflowDeps)(nil) func (d *mprWorkflowDeps) SerializeWorkflowActivity(a workflows.WorkflowActivity) bson.D { - return mpr.SerializeWorkflowActivity(a) + return mpr.SerializeWorkflowActivity(a, d.backend.useCallMicroflowActivityName()) } func (d *mprWorkflowDeps) SaveUnit(unitID string, contents []byte) error { diff --git a/sdk/mpr/workflow_write_test.go b/sdk/mpr/workflow_write_test.go index 3d9393188..f16763990 100644 --- a/sdk/mpr/workflow_write_test.go +++ b/sdk/mpr/workflow_write_test.go @@ -366,3 +366,39 @@ func TestSerializeWorkflowFlow_RoundtripFromFixture(t *testing.T) { t.Errorf("roundtrip last activity = %T, want *workflows.EndWorkflowActivity", last) } } + +// TestRenameCallMicroflowTypeBSON verifies the version-gated $Type rewrite in the +// legacy engine (FINDINGS #39): a CallMicroflowTask nested inside a Flow's +// activities array is renamed to CallMicroflowActivity only when useActivity is set. +func TestRenameCallMicroflowTypeBSON(t *testing.T) { + build := func() bson.D { + return bson.D{ + {Key: "$Type", Value: "Workflows$Workflow"}, + {Key: "Flow", Value: bson.D{ + {Key: "$Type", Value: "Workflows$Flow"}, + {Key: "Activities", Value: bson.A{ + int32(3), + bson.D{{Key: "$Type", Value: "Workflows$CallMicroflowTask"}, {Key: "Name", Value: "Call"}}, + bson.D{{Key: "$Type", Value: "Workflows$EndWorkflowActivity"}}, + }}, + }}, + } + } + typeOfActivity := func(d bson.D) string { + flow := d[1].Value.(bson.D) + acts := flow[1].Value.(bson.A) + return acts[1].(bson.D)[0].Value.(string) + } + + off := build() + renameCallMicroflowTypeBSON(off, false) + if got := typeOfActivity(off); got != "Workflows$CallMicroflowTask" { + t.Errorf("pre-11.9: activity $Type = %q, want Workflows$CallMicroflowTask", got) + } + + on := build() + renameCallMicroflowTypeBSON(on, true) + if got := typeOfActivity(on); got != "Workflows$CallMicroflowActivity" { + t.Errorf("11.9+: activity $Type = %q, want Workflows$CallMicroflowActivity", got) + } +} diff --git a/sdk/mpr/writer_workflow.go b/sdk/mpr/writer_workflow.go index 12c43d642..eed054c67 100644 --- a/sdk/mpr/writer_workflow.go +++ b/sdk/mpr/writer_workflow.go @@ -109,9 +109,43 @@ func (w *Writer) serializeWorkflow(wf *workflows.Workflow) ([]byte, error) { // NOTE: OverviewPage was deleted in Mendix 9.11.0 — do not serialize it. // NOTE: AllowedModuleRoles is not present in Studio Pro BSON — omitted. + pv := w.reader.ProjectVersion() + renameCallMicroflowTypeBSON(doc, pv != nil && pv.IsAtLeast(11, 9)) return marshalUnitIDFirst(doc) } +// renameCallMicroflowTypeBSON rewrites every "Workflows$CallMicroflowTask" $Type +// in a serialized workflow tree to the 11.9+ "Workflows$CallMicroflowActivity" +// name when useActivity is set. Mendix 11.9 (WOR-2802) split MicroflowBasedActivity +// into CallMicroflowActivity + AIAgentTaskActivity; writing the pre-11.9 name to an +// 11.9+ project makes the runtime fail to load the whole model (FINDINGS #39). The +// modelsdk engine does the same via applyCallMicroflowStorageName. +func renameCallMicroflowTypeBSON(v any, useActivity bool) { + if !useActivity { + return + } + renameCallMicroflowWalk(v) +} + +func renameCallMicroflowWalk(v any) { + switch t := v.(type) { + case bson.D: + for i := range t { + if t[i].Key == "$Type" { + if s, ok := t[i].Value.(string); ok && s == "Workflows$CallMicroflowTask" { + t[i].Value = "Workflows$CallMicroflowActivity" + } + continue + } + renameCallMicroflowWalk(t[i].Value) + } + case bson.A: + for i := range t { + renameCallMicroflowWalk(t[i]) + } + } +} + // serializeWorkflowStringTemplate creates a minimal Mendix StringTemplate BSON structure for workflows. func serializeWorkflowStringTemplate(text string) bson.D { return bson.D{ @@ -234,8 +268,10 @@ func serializeWorkflowFlow(flow *workflows.Flow) bson.D { // SerializeWorkflowActivity dispatches to the correct activity serializer. // Exported for use by the ALTER WORKFLOW executor. -func SerializeWorkflowActivity(act workflows.WorkflowActivity) bson.D { - return serializeWorkflowActivity(act) +func SerializeWorkflowActivity(act workflows.WorkflowActivity, useCallMicroflowActivityName bool) bson.D { + d := serializeWorkflowActivity(act) + renameCallMicroflowTypeBSON(d, useCallMicroflowActivityName) + return d } // serializeWorkflowActivity dispatches to the correct serializer. From 6f0750e31e322644d9faf0326f223df1cef475af Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:33:21 +0000 Subject: [PATCH 03/21] fix(settings): reject unconfigured DefaultLanguageCode in ALTER SETTINGS LANGUAGE (FINDINGS #6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `alter settings LANGUAGE DefaultLanguageCode = '<code>'` accepted any string. A code not configured in the project (e.g. 'nl_NL' on an en_US-only project) was written, reported success, and the *next* `mx check` died with an unhandled NullReferenceException — never a model error, so the corruption was invisible until a later command. Validate the code against the project's configured languages (ps.Language.Languages) before writing; reject with the available codes and a Studio Pro hint. Skipped when the language list is unavailable (empty) to avoid false rejections. Verified end to end on the vendored fixture: 'nl_NL' rejected, 'en_US' accepted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- mdl/executor/cmd_settings.go | 27 ++++++++++++++ mdl/executor/cmd_settings_language_test.go | 41 ++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 mdl/executor/cmd_settings_language_test.go diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index b83cdfdb1..b890780b0 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -4,6 +4,7 @@ package executor import ( "fmt" + "sort" "strconv" "strings" @@ -220,6 +221,9 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { valStr := settingsValueToString(val) switch key { case "DefaultLanguageCode": + if err := validateLanguageCode(ps.Language, valStr); err != nil { + return err + } ps.Language.DefaultLanguageCode = valStr default: return mdlerrors.NewUnsupported("unknown language setting: " + key) @@ -267,6 +271,29 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { return nil } +// validateLanguageCode rejects a DefaultLanguageCode that is not one of the +// project's configured languages. Mendix has no such guard: `alter settings +// LANGUAGE` would accept e.g. 'nl_NL' on an en_US-only project, the write would +// report success, and the *next* `mx check` would die with an unhandled +// NullReferenceException rather than a model error (FINDINGS #6). Skipped when the +// project's language list is empty (unavailable) to avoid false rejections. +func validateLanguageCode(ls *model.LanguageSettings, code string) error { + if ls == nil || len(ls.Languages) == 0 { + return nil + } + avail := make([]string, 0, len(ls.Languages)) + for _, l := range ls.Languages { + if l.Code == code { + return nil + } + avail = append(avail, l.Code) + } + sort.Strings(avail) + return mdlerrors.NewValidationf( + "language %q is not configured in this project (available: %s) — add it in Studio Pro (Project ▸ Settings ▸ Languages) before making it the default", + code, strings.Join(avail, ", ")) +} + func alterSettingsConfiguration(ctx *ExecContext, ps *model.ProjectSettings, stmt *ast.AlterSettingsStmt) error { if ps.Configuration == nil { return mdlerrors.NewNotFound("settings section", "configuration") diff --git a/mdl/executor/cmd_settings_language_test.go b/mdl/executor/cmd_settings_language_test.go new file mode 100644 index 000000000..428d6e38e --- /dev/null +++ b/mdl/executor/cmd_settings_language_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" +) + +// TestValidateLanguageCode guards FINDINGS #6: setting DefaultLanguageCode to a +// language not configured in the project must be rejected up front, because the +// write otherwise "succeeds" and the next mx check dies with a NullReferenceException. +func TestValidateLanguageCode(t *testing.T) { + ls := &model.LanguageSettings{ + Languages: []model.Language{{Code: "en_US"}, {Code: "de_DE"}}, + } + + // Configured code → accepted. + if err := validateLanguageCode(ls, "en_US"); err != nil { + t.Errorf("validateLanguageCode(en_US) = %v, want nil", err) + } + + // Unconfigured code → rejected, and the message lists the available codes. + err := validateLanguageCode(ls, "nl_NL") + if err == nil { + t.Fatalf("validateLanguageCode(nl_NL) = nil, want rejection") + } + if msg := err.Error(); !strings.Contains(msg, "en_US") || !strings.Contains(msg, "de_DE") { + t.Errorf("error message %q should list available codes en_US, de_DE", msg) + } + + // No language list available → skip validation (avoid false rejection). + if err := validateLanguageCode(&model.LanguageSettings{}, "nl_NL"); err != nil { + t.Errorf("validateLanguageCode with empty list = %v, want nil (skip)", err) + } + if err := validateLanguageCode(nil, "nl_NL"); err != nil { + t.Errorf("validateLanguageCode(nil) = %v, want nil (skip)", err) + } +} From 3c9c7fb247c46f66fec53238bc8b50a52624cb29 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:35:26 +0000 Subject: [PATCH 04/21] =?UTF-8?q?fix(grammar):=20allow=20V3=20as=20an=20id?= =?UTF-8?q?entifier=20=E2=80=94=20remove=20dead=20lexer=20token=20(FINDING?= =?UTF-8?q?S=20#2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lexer defined `V3: V '3';` (a "syntax version V3" marker) that no parser rule or visitor ever consumed — a dead token. Its only effect was to tokenize any bare `V3` in the input as the V3 token instead of IDENTIFIER, so `V3` was unusable as an attribute name, property name, or page binding while `V1`, `V2`, `V4`, `V5` (never tokens) all worked. Quoting (`"V3"`) was the only workaround. Removing the unused token lets `V3` tokenize as a normal IDENTIFIER. Verified: an attribute `V3 = 'c'` in a create activity and `Attribute: V3` in a page binding both parse, alongside the quoted form; grammar regenerated (`make grammar`); visitor and executor suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- mdl/grammar/MDLLexer.g4 | 1 - 1 file changed, 1 deletion(-) diff --git a/mdl/grammar/MDLLexer.g4 b/mdl/grammar/MDLLexer.g4 index c61c7234a..93bbd5a69 100644 --- a/mdl/grammar/MDLLexer.g4 +++ b/mdl/grammar/MDLLexer.g4 @@ -640,7 +640,6 @@ WITHOUT: W I T H O U T; DRY: D R Y; RUN: R U N; WIDGETTYPE: W I D G E T T Y P E; -V3: V '3'; // Syntax version V3 // ============================================================================= // BUSINESS EVENTS / SETTINGS KEYWORDS From 2099bbe1b04a3498c8a944ff7061ede21258bcce Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:39:17 +0000 Subject: [PATCH 05/21] fix(workflows): normalize with(...) param name to bare form (FINDINGS #41) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow `call microflow ... with (Name = ...)` mapping accepted a qualified (Mod.Flow.Param) or quoted ("Param") name via the QualifiedName grammar rule and stored its raw text. The executor then re-qualifies with `mfQN + "." + name`, so: - qualified input Mod.Flow.Param → stored Mod.Flow.Mod.Flow.Param → null ParameterId → runtime fails to load the model - quoted input "Param" → stored Mod.Flow."Param" → CE1613 - bare input Param → stored Mod.Flow.Param → correct Only the bare form worked — the one place MDL's "always quote identifiers" habit is actively wrong. Normalize the mapping name in the visitor to the bare, unquoted last segment so all three spellings converge on the correct single-qualified stored form. Verified end to end: bare, quoted, and fully-qualified inputs all store `MyFirstModule.ACT_Do.Item`. Unit test covers the normalizer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- mdl/visitor/visitor_workflow.go | 20 ++++++++++++++++++-- mdl/visitor/visitor_workflow_test.go | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/mdl/visitor/visitor_workflow.go b/mdl/visitor/visitor_workflow.go index 731c10d13..eb39e8b05 100644 --- a/mdl/visitor/visitor_workflow.go +++ b/mdl/visitor/visitor_workflow.go @@ -3,6 +3,8 @@ package visitor import ( + "strings" + "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/grammar/parser" ) @@ -562,7 +564,7 @@ func buildWorkflowCallMicroflow(ctx parser.IWorkflowCallMicroflowStmtContext) *a for _, pmCtx := range cmCtx.AllWorkflowParameterMapping() { pmCtx2 := pmCtx.(*parser.WorkflowParameterMappingContext) mapping := ast.WorkflowParameterMappingNode{ - Parameter: pmCtx2.QualifiedName().GetText(), + Parameter: bareWorkflowParameterName(pmCtx2.QualifiedName().GetText()), Expression: unquoteString(pmCtx2.STRING_LITERAL().GetText()), } node.ParameterMappings = append(node.ParameterMappings, mapping) @@ -576,6 +578,20 @@ func buildWorkflowCallMicroflow(ctx parser.IWorkflowCallMicroflowStmtContext) *a return node } +// bareWorkflowParameterName normalizes the name in a workflow `with (Name = ...)` +// mapping to the bare, unquoted last segment. A workflow call-microflow/call-workflow +// maps to the target's parameter by BARE name: a fully-qualified name +// (Mod.Flow.Param) writes a null ParameterId and makes the runtime fail to load the +// model, and a quoted name ("Param") is looked up verbatim → CE1613. Studio Pro uses +// the bare name, and this is the one place MDL's "always quote identifiers" habit is +// actively wrong — so accept all three spellings and normalize (FINDINGS #41). +func bareWorkflowParameterName(raw string) string { + if i := strings.LastIndex(raw, "."); i >= 0 { + raw = raw[i+1:] + } + return unquoteIdentifier(strings.TrimSpace(raw)) +} + // buildWorkflowCallWorkflow builds a WorkflowCallWorkflowNode. func buildWorkflowCallWorkflow(ctx parser.IWorkflowCallWorkflowStmtContext) *ast.WorkflowCallWorkflowNode { cwCtx := ctx.(*parser.WorkflowCallWorkflowStmtContext) @@ -591,7 +607,7 @@ func buildWorkflowCallWorkflow(ctx parser.IWorkflowCallWorkflowStmtContext) *ast for _, pmCtx := range cwCtx.AllWorkflowParameterMapping() { pmCtx2 := pmCtx.(*parser.WorkflowParameterMappingContext) mapping := ast.WorkflowParameterMappingNode{ - Parameter: pmCtx2.QualifiedName().GetText(), + Parameter: bareWorkflowParameterName(pmCtx2.QualifiedName().GetText()), Expression: unquoteString(pmCtx2.STRING_LITERAL().GetText()), } node.ParameterMappings = append(node.ParameterMappings, mapping) diff --git a/mdl/visitor/visitor_workflow_test.go b/mdl/visitor/visitor_workflow_test.go index 4cb9d6b28..2a2298098 100644 --- a/mdl/visitor/visitor_workflow_test.go +++ b/mdl/visitor/visitor_workflow_test.go @@ -1229,3 +1229,21 @@ func TestAlterWorkflow_SetActivityDueDate(t *testing.T) { t.Errorf("Expected Value 'PT72H', got %q", op.Value) } } + +// TestBareWorkflowParameterName guards FINDINGS #41: a workflow `with (Name = ...)` +// param mapping must normalize to the bare, unquoted last segment, because a +// qualified name writes a null ParameterId (unloadable model) and a quoted name is +// looked up verbatim (CE1613). Studio Pro uses the bare name. +func TestBareWorkflowParameterName(t *testing.T) { + cases := map[string]string{ + "Timesheet": "Timesheet", // bare — already correct + `"Timesheet"`: "Timesheet", // quoted — strip quotes + "TimeReg.ACT_ApproveWeek.Timesheet": "Timesheet", // fully qualified — last segment + `TimeReg.ACT_ApproveWeek."Timesheet"`: "Timesheet", // qualified + quoted last segment + } + for in, want := range cases { + if got := bareWorkflowParameterName(in); got != want { + t.Errorf("bareWorkflowParameterName(%q) = %q, want %q", in, got, want) + } + } +} From 53cb961660ac4b33601980750736fb0d7623e083 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:47:31 +0000 Subject: [PATCH 06/21] =?UTF-8?q?fix(mdl):=20parser=20ergonomics=20batch?= =?UTF-8?q?=20=E2=80=94=20index/role=20quoting,=20sort-by,=20assoc=20sugar?= =?UTF-8?q?,=20ADD=20doc=20(FINDINGS=20#3/#5/#13/#14/#4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five surprising parser rejects / silent-wrong writes where quoting or a keyword behaved inconsistently with the rest of MDL: - #3 index name can't be quoted: indexDefinition accepted only IDENTIFIER for the name → `index "idx_x" (Col)` was a parse error. Accept QUOTED_IDENTIFIER too (the name is advisory and discarded, as before). - #5 role-name quoting inconsistent: DESCRIBE USER ROLE required quotes, DROP USER ROLE rejected them. Both now accept bare and quoted names; visitors handle both. - #13 `sort by` quoted attribute stored a nonsense reference: a quoted qualified attribute (`sort by "Mod"."Entity"."Code"`) kept the quotes and only failed on write ("attribute does not belong to entity"). buildSortColumnMicroflow now unquotes each segment (bare dotted form), matching the SORT() list-op path. - #14 `DataSource: ASSOCIATION $currentObject/…` didn't parse (keyword + sugar were mutually exclusive). Added the combined grammar branch; the existing VARIABLE&&SLASH visitor branch already handles it correctly. - #4 doc: MDL spec README showed `alter entity … add (Attr: type)`, which the parser rejects. Corrected to `add attribute Attr: type`. Grammar regenerated (`make grammar`). Verified end to end: quoted index name, quoted qualified sort stored as `M.Thing.Code`, describe/drop role in both forms, and `ASSOCIATION $currentObject/…` all parse/store correctly. Tests added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- docs/05-mdl-specification/README.md | 2 +- mdl/grammar/domains/MDLCatalog.g4 | 2 +- mdl/grammar/domains/MDLDomainModel.g4 | 2 +- mdl/grammar/domains/MDLPage.g4 | 1 + mdl/grammar/domains/MDLSecurity.g4 | 2 +- mdl/visitor/parser_batch_findings_test.go | 68 +++++++++++++++++++++ mdl/visitor/visitor_microflow_statements.go | 10 ++- mdl/visitor/visitor_query.go | 10 ++- mdl/visitor/visitor_security.go | 12 +++- 9 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 mdl/visitor/parser_batch_findings_test.go diff --git a/docs/05-mdl-specification/README.md b/docs/05-mdl-specification/README.md index 80f375772..852126e81 100644 --- a/docs/05-mdl-specification/README.md +++ b/docs/05-mdl-specification/README.md @@ -30,7 +30,7 @@ describe entity Module.EntityName; create persistent entity Module.Name ( AttrName: type [not null] [unique] [default value] ); -alter entity Module.Name add (NewAttr: string(200)); +alter entity Module.Name add attribute NewAttr: string(200); drop entity Module.Name; -- Microflows diff --git a/mdl/grammar/domains/MDLCatalog.g4 b/mdl/grammar/domains/MDLCatalog.g4 index dafc8a9db..d367099a1 100644 --- a/mdl/grammar/domains/MDLCatalog.g4 +++ b/mdl/grammar/domains/MDLCatalog.g4 @@ -148,7 +148,7 @@ describeStatement | DESCRIBE JAVASCRIPT ACTION qualifiedName | DESCRIBE MODULE identifierOrKeyword (WITH ALL)? // DESCRIBE MODULE Name [WITH ALL] - optionally include all objects | DESCRIBE MODULE ROLE qualifiedName // DESCRIBE MODULE ROLE Module.RoleName - | DESCRIBE USER ROLE STRING_LITERAL // DESCRIBE USER ROLE 'Administrator' + | DESCRIBE USER ROLE (STRING_LITERAL | identifierOrKeyword) // DESCRIBE USER ROLE 'Administrator' | Administrator | DESCRIBE DEMO USER STRING_LITERAL // DESCRIBE DEMO USER 'demo_admin' | DESCRIBE ODATA CLIENT qualifiedName // DESCRIBE ODATA CLIENT Module.ServiceName | DESCRIBE ODATA SERVICE qualifiedName // DESCRIBE ODATA SERVICE Module.ServiceName diff --git a/mdl/grammar/domains/MDLDomainModel.g4 b/mdl/grammar/domains/MDLDomainModel.g4 index 02a9f7e3e..8d0fd950b 100644 --- a/mdl/grammar/domains/MDLDomainModel.g4 +++ b/mdl/grammar/domains/MDLDomainModel.g4 @@ -143,7 +143,7 @@ nonListDataType // The optional ON reads SQL-like: `INDEX idx_name ON (Col1, Col2)`. The bare // form `INDEX idx_name (Col1, Col2)` (and anonymous `INDEX (Col1)`) still parse. indexDefinition - : IDENTIFIER? ON? LPAREN indexAttributeList RPAREN + : (IDENTIFIER | QUOTED_IDENTIFIER)? ON? LPAREN indexAttributeList RPAREN ; indexAttributeList diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index 5a6c09493..9d9f8710b 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -407,6 +407,7 @@ dataSourceExprV3 | MICROFLOW qualifiedName microflowArgsV3? // MICROFLOW Module.Flow | NANOFLOW qualifiedName microflowArgsV3? // NANOFLOW Module.Flow | ASSOCIATION associationPathV3 // ASSOCIATION Module.Assoc (explicit form) + | ASSOCIATION VARIABLE SLASH associationPathV3 // ASSOCIATION $currentObject/Module.Assoc (keyword + sugar) | SELECTION (IDENTIFIER | QUOTED_IDENTIFIER) // SELECTION widgetName ("name" if reserved) ; diff --git a/mdl/grammar/domains/MDLSecurity.g4 b/mdl/grammar/domains/MDLSecurity.g4 index 6b3da99d9..bcd1c1e45 100644 --- a/mdl/grammar/domains/MDLSecurity.g4 +++ b/mdl/grammar/domains/MDLSecurity.g4 @@ -30,7 +30,7 @@ alterUserRoleStatement ; dropUserRoleStatement - : DROP USER ROLE identifierOrKeyword + : DROP USER ROLE (identifierOrKeyword | STRING_LITERAL) ; grantEntityAccessStatement diff --git a/mdl/visitor/parser_batch_findings_test.go b/mdl/visitor/parser_batch_findings_test.go new file mode 100644 index 000000000..0dd0431ec --- /dev/null +++ b/mdl/visitor/parser_batch_findings_test.go @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// TestRetrieveSortByQuotedQualified guards FINDINGS #13: a quoted, qualified +// `sort by` attribute in a RETRIEVE must store the bare dotted form. Keeping the +// quotes produced a reference that only failed on write ("attribute does not +// belong to entity"). +func TestRetrieveSortByQuotedQualified(t *testing.T) { + input := `create microflow M.DS () returns list of M.Thing as $rows +begin + retrieve $rows from M.Thing sort by "M"."Thing"."Code" asc; + return $rows; +end;` + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got string + for _, s := range mf.Body { + if r, ok := s.(*ast.RetrieveStmt); ok && len(r.SortColumns) > 0 { + got = r.SortColumns[0].Attribute + } + } + if got != "M.Thing.Code" { + t.Errorf("sort attribute = %q, want %q (unquoted dotted)", got, "M.Thing.Code") + } +} + +// TestUserRoleQuotingConsistency guards FINDINGS #5: DESCRIBE and DROP USER ROLE +// both accept bare and quoted names, so the two commands are consistent. +func TestUserRoleQuotingConsistency(t *testing.T) { + cases := []struct { + input string + wantName string + }{ + {"describe user role Administrator;", "Administrator"}, + {"describe user role 'Administrator';", "Administrator"}, + {"drop user role User;", "User"}, + {"drop user role 'User';", "User"}, + } + for _, tc := range cases { + prog, errs := Build(tc.input) + if len(errs) > 0 { + t.Errorf("%q: unexpected parse errors: %v", tc.input, errs) + continue + } + switch s := prog.Statements[0].(type) { + case *ast.DescribeStmt: + if s.Name.Name != tc.wantName { + t.Errorf("%q: describe role name = %q, want %q", tc.input, s.Name.Name, tc.wantName) + } + case *ast.DropUserRoleStmt: + if s.Name != tc.wantName { + t.Errorf("%q: drop role name = %q, want %q", tc.input, s.Name, tc.wantName) + } + default: + t.Errorf("%q: unexpected statement type %T", tc.input, prog.Statements[0]) + } + } +} diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index fbcaf5c35..c74bff7af 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -1315,11 +1315,15 @@ func buildSortColumnMicroflow(ctx parser.ISortColumnContext) *ast.SortColumnDef Order: "ASC", // Default to ASC } - // Get attribute name from QualifiedName or IDENTIFIER + // Get attribute name from QualifiedName or IDENTIFIER. Strip quotes from each + // segment: `sort by "Mod"."Entity"."Code"` must store the bare dotted form — + // keeping the quotes produced a nonsense reference that only failed on write + // ("attribute does not belong to entity"), unlike everywhere else where quoting + // is safe (FINDINGS #13). if qn := colCtx.QualifiedName(); qn != nil { - col.Attribute = qn.GetText() + col.Attribute = unquoteQualifiedName(qn.GetText()) } else if id := colCtx.IDENTIFIER(); id != nil { - col.Attribute = id.GetText() + col.Attribute = unquoteIdentifier(id.GetText()) } // Get sort order diff --git a/mdl/visitor/visitor_query.go b/mdl/visitor/visitor_query.go index 829a4d1e3..64d8727c5 100644 --- a/mdl/visitor/visitor_query.go +++ b/mdl/visitor/visitor_query.go @@ -708,10 +708,16 @@ func (b *Builder) ExitDescribeStatement(ctx *parser.DescribeStatementContext) { return } - // Handle DESCRIBE USER ROLE 'Name' (uses STRING_LITERAL) + // Handle DESCRIBE USER ROLE 'Name' | Name (quoted or bare — DROP accepts both + // too, so the two commands are consistent; FINDINGS #5). if ctx.USER() != nil && ctx.ROLE() != nil { + roleName := "" if sl := ctx.STRING_LITERAL(); sl != nil { - roleName := unquoteString(sl.GetText()) + roleName = unquoteString(sl.GetText()) + } else if iok := ctx.IdentifierOrKeyword(); iok != nil { + roleName = identifierOrKeywordText(iok) + } + if roleName != "" { b.statements = append(b.statements, &ast.DescribeStmt{ ObjectType: ast.DescribeUserRole, Name: ast.QualifiedName{Module: roleName, Name: roleName}, diff --git a/mdl/visitor/visitor_security.go b/mdl/visitor/visitor_security.go index 30747c979..fd91ff405 100644 --- a/mdl/visitor/visitor_security.go +++ b/mdl/visitor/visitor_security.go @@ -84,10 +84,16 @@ func (b *Builder) ExitAlterUserRoleStatement(ctx *parser.AlterUserRoleStatementC // ExitDropUserRoleStatement handles DROP USER ROLE Name func (b *Builder) ExitDropUserRoleStatement(ctx *parser.DropUserRoleStatementContext) { + name := "" if iok := ctx.IdentifierOrKeyword(); iok != nil { - b.statements = append(b.statements, &ast.DropUserRoleStmt{ - Name: identifierOrKeywordText(iok), - }) + name = identifierOrKeywordText(iok) + } else if sl := ctx.STRING_LITERAL(); sl != nil { + // Quoted form (FINDINGS #5): DROP USER ROLE 'User' — matches DESCRIBE, which + // also accepts quotes. Bare and quoted are now consistent across both. + name = unquoteString(sl.GetText()) + } + if name != "" { + b.statements = append(b.statements, &ast.DropUserRoleStmt{Name: name}) } } From cdaab9ffdc42da5b9339a23b5b79210ff21e83eb Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:55:27 +0000 Subject: [PATCH 07/21] fix(workflows,lint,docs): describe param round-trip, SEC005 hint, skill gaps (FINDINGS #42/#36/#23/#48/#47) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #42 DESCRIBE WORKFLOW dropped the `with (...)` param mappings: the modelsdk read path built a CallMicroflowTask with only Microflow+Outcomes and never populated ParameterMappings, so describe→drop→exec silently lost the mapping (which nothing then reports). Added microflowParamMappingsFromGen and wired it into both the CallMicroflowTask and CallMicroflowActivity read cases; the legacy sdk/mpr parser already read them. Round-trip test added; verified `describe workflow` now emits `call microflow M.ACT_Do with (Item = '$workflowContext')`. - #36 SEC005 lint suggested `ALTER PROJECT SECURITY STRICT MODE ON`, a statement the parser doesn't implement. Strict mode is Studio Pro-only — the suggestion now says so instead of naming an unrunnable command. - #23 documented `create or modify association` (the idempotent form) in the domain-model skill: plain `create association` is not idempotent and its failure aborts the rest of the script. - #48 documented `set task outcome $Task '<Outcome>'` (there is no `complete task`) and the other workflow task statements in write-workflows. - #47 documented that System-module enumerations are read from the runtime, not the .mpr, so mxcli can't resolve them — constrain on an attribute instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/lint-rules/sec_strict_mode.star | 3 +- .../skills/mendix/generate-domain-model.md | 11 +++ .claude/skills/mendix/write-workflows.md | 23 +++++++ mdl/backend/modelsdk/workflow_read.go | 23 +++++++ .../modelsdk/workflow_storagename_test.go | 67 +++++++++++++++++++ 5 files changed, 126 insertions(+), 1 deletion(-) diff --git a/.claude/lint-rules/sec_strict_mode.star b/.claude/lint-rules/sec_strict_mode.star index aa97ae30c..c9f18c05f 100644 --- a/.claude/lint-rules/sec_strict_mode.star +++ b/.claude/lint-rules/sec_strict_mode.star @@ -27,5 +27,6 @@ def check(): return [violation( message="Strict mode is disabled. This weakens XPath constraint enforcement and is relevant to CVE-2023-23835.", location=location(module="", document_type="security", document_name="ProjectSecurity"), - suggestion="ALTER PROJECT SECURITY STRICT MODE ON", + # mxcli/MDL cannot toggle strict mode — it is a Studio Pro-only setting. + suggestion="Enable strict mode in Studio Pro: Project Security > Enable 'Check security' and turn on strict-mode XPath validation (not settable via MDL).", )] diff --git a/.claude/skills/mendix/generate-domain-model.md b/.claude/skills/mendix/generate-domain-model.md index 4bff018b0..1e62b7acd 100644 --- a/.claude/skills/mendix/generate-domain-model.md +++ b/.claude/skills/mendix/generate-domain-model.md @@ -388,6 +388,17 @@ delete_behavior DELETE_BUT_KEEP_REFERENCES comment 'Additional documentation'; ``` +**Idempotency**: plain `create association` is **not** idempotent — re-running it +errors with `association already exists`, which aborts the rest of the script (and +any associations defined *after* it are never created). Write **`create or modify +association`** from the first draft — same clauses, but re-running is a no-op: + +```sql +create or modify association Module.Child_Parent +from Module.Child to Module.Parent +type reference; +``` + **Association Types**: - `reference` - One-to-one or many-to-one (foreign key on FROM entity) - `ReferenceSet` - One-to-many or many-to-many (collection) diff --git a/.claude/skills/mendix/write-workflows.md b/.claude/skills/mendix/write-workflows.md index a7d80a4e8..da27bcb90 100644 --- a/.claude/skills/mendix/write-workflows.md +++ b/.claude/skills/mendix/write-workflows.md @@ -152,6 +152,29 @@ Studio-Pro-authored workflow, and `describe → drop → exec` reproduces a work that builds. (The implicit start/end activities are omitted, as they are re-synthesised on create.) +## Microflow statements for workflow tasks + +These run **inside a microflow** (not in the workflow body) and drive a running +workflow / its tasks. They are easy to miss — there is no `complete task`: + +- `set task outcome $Task 'Approve';` — completes a `System.WorkflowUserTask` with a + named outcome. This is how a microflow (e.g. a task page's button) finishes a task + and does the domain work; the outcome branches still record which one was chosen. +- `open user task $Task`, `notify workflow $Wf`, `lock workflow $Wf`, and + `workflow operation abort|pause|restart|retry|continue $Wf` are also statements. + +A common shape: the task page's buttons call a microflow that does the change and +then `set task outcome $Task '<Outcome>'`, leaving the workflow's outcome branch +bodies empty. + +## System-module documents are read from the runtime, not the .mpr + +`describe enumeration System.WorkflowUserTaskState` and `show enumerations in System` +return nothing — the System module's **enumerations** are not in the project file, so +mxcli cannot resolve them. Constrain on an attribute instead (`[EndTime = empty]` +selects open tasks) rather than naming a System enum value. System **entities** are +documented in `system-module.md`. + ## Platform rules - A user task needs a **task page** to be useful; without one Mendix flags the diff --git a/mdl/backend/modelsdk/workflow_read.go b/mdl/backend/modelsdk/workflow_read.go index 111d09209..68ac04a57 100644 --- a/mdl/backend/modelsdk/workflow_read.go +++ b/mdl/backend/modelsdk/workflow_read.go @@ -119,11 +119,13 @@ func workflowActivityFromGen(el element.Element) workflows.WorkflowActivity { t := &workflows.CallMicroflowTask{Microflow: a.MicroflowQualifiedName()} setWfBase(&t.BaseWorkflowActivity, a.ID(), a.Name(), a.Caption(), a.Annotation(), "Workflows$CallMicroflowTask") t.Outcomes = conditionOutcomesFromGen(a.OutcomesItems()) + t.ParameterMappings = microflowParamMappingsFromGen(a.ParameterMappingsItems()) return t case *genWf.CallMicroflowActivity: t := &workflows.CallMicroflowTask{Microflow: a.MicroflowQualifiedName()} setWfBase(&t.BaseWorkflowActivity, a.ID(), a.Name(), a.Caption(), a.Annotation(), "Workflows$CallMicroflowActivity") t.Outcomes = conditionOutcomesFromGen(a.OutcomesItems()) + t.ParameterMappings = microflowParamMappingsFromGen(a.ParameterMappingsItems()) return t case *genWf.CallWorkflowActivity: t := &workflows.CallWorkflowActivity{ @@ -233,6 +235,27 @@ func userTaskOutcomesFromGen(items []element.Element) []*workflows.UserTaskOutco // conditionOutcomesFromGen converts gen condition outcomes to semantic ones, // mirroring the legacy parseConditionOutcomes dispatch. +// microflowParamMappingsFromGen reads a call-microflow activity's parameter +// mappings back into the semantic model. Without this, DESCRIBE WORKFLOW dropped +// the `with (...)` clause even though it was stored — a describe→drop→exec cycle +// silently lost the mapping (FINDINGS #42). +func microflowParamMappingsFromGen(items []element.Element) []*workflows.ParameterMapping { + var out []*workflows.ParameterMapping + for _, el := range items { + pm, ok := el.(*genWf.MicroflowCallParameterMapping) + if !ok { + continue + } + m := &workflows.ParameterMapping{ + Parameter: pm.ParameterQualifiedName(), + Expression: pm.Expression(), + } + m.ID = model.ID(pm.ID()) + out = append(out, m) + } + return out +} + func conditionOutcomesFromGen(items []element.Element) []workflows.ConditionOutcome { var out []workflows.ConditionOutcome for _, el := range items { diff --git a/mdl/backend/modelsdk/workflow_storagename_test.go b/mdl/backend/modelsdk/workflow_storagename_test.go index 055c79e2d..2aa254333 100644 --- a/mdl/backend/modelsdk/workflow_storagename_test.go +++ b/mdl/backend/modelsdk/workflow_storagename_test.go @@ -89,3 +89,70 @@ func TestUseCallMicroflowActivityName_Pre119(t *testing.T) { t.Errorf("fixture is Mendix 11.6.6 (< 11.9); useCallMicroflowActivityName() = true, want false") } } + +// TestWorkflowParameterMappingRoundTrip guards FINDINGS #42: a call-microflow +// activity's parameter mappings must survive create → read, so DESCRIBE WORKFLOW +// re-emits the `with (...)` clause instead of silently dropping it. +func TestWorkflowParameterMappingRoundTrip(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + wf := &workflows.Workflow{ + ContainerID: mod.ID, + Name: "PmFlow", + WorkflowName: "Pm Flow", + Parameter: &workflows.WorkflowParameter{EntityRef: "MyFirstModule.Ctx"}, + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{ + &workflows.StartWorkflowActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "Start"}}, + &workflows.CallMicroflowTask{ + BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "Call", Caption: "Call"}, + Microflow: "MyFirstModule.ACT_Do", + ParameterMappings: []*workflows.ParameterMapping{ + {Parameter: "Item", Expression: "$workflowContext"}, + }, + }, + &workflows.EndWorkflowActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{Name: "End"}}, + }}, + } + if err := b.CreateWorkflow(wf); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b2.Disconnect() }) + all, err := b2.ListWorkflows() + if err != nil { + t.Fatalf("ListWorkflows: %v", err) + } + var cm *workflows.CallMicroflowTask + for _, w := range all { + if w.Name != "PmFlow" || w.Flow == nil { + continue + } + for _, act := range w.Flow.Activities { + if c, ok := act.(*workflows.CallMicroflowTask); ok { + cm = c + } + } + } + if cm == nil { + t.Fatal("call-microflow activity not found after round-trip") + } + if len(cm.ParameterMappings) != 1 { + t.Fatalf("ParameterMappings count = %d, want 1 (mapping dropped on read)", len(cm.ParameterMappings)) + } + if cm.ParameterMappings[0].Expression != "$workflowContext" { + t.Errorf("mapping expression = %q, want %q", cm.ParameterMappings[0].Expression, "$workflowContext") + } +} From 9830018f065747bed4490205b17444a13ac24006 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 15:59:49 +0000 Subject: [PATCH 08/21] fix(microflows): reject create/change member ref to a non-existent association (FINDINGS #51) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `create`/`change` activity assigning a one-qualifier member `Module.Name` that is not an existing association was written as an *attribute* reference. A one-qualifier name can never be a valid attribute (attributes are bare or Module.Entity.Attribute), so the result was an invalid AttributeIdentifier and the next `mx check` could not LOAD the project at all (StorageLoadException), even though `mxcli exec` reported success. This commonly followed a non-idempotent `create association` that had failed earlier in the script, leaving the association absent while a later `create` still referenced it — a green exec into an unloadable .mpr. In resolveMemberChange, when the domain model is available and a one-dot member is not found in the module's associations (or cross-associations), reject it with an actionable error ("create the association first ...") instead of serializing an Attribute. Associations created earlier in the same script are visible via GetDomainModel, so this does not false-positive on same-script associations; two-dot qualified attributes (Module.Entity.Attribute) are still allowed. Verified end to end: nonexistent association → clear error; same-script association → succeeds; qualified attribute → succeeds. Regression test + repro added; full executor suite (incl. cross-module association tests) passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .../264-create-member-unknown-association.mdl | 50 +++++++++++++++++++ .../cmd_microflows_builder_actions.go | 20 ++++++-- .../cmd_microflows_member_identifier_test.go | 34 +++++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 mdl-examples/bug-tests/264-create-member-unknown-association.mdl diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index d8e02733d..35c8b378d 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -23,6 +23,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | `describe` shows `$var = list operation %T;` (with type name) | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatListOperation()` | Add `case *microflows.XxxOperation:` before the `default` | | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | +| `mx check` fails to LOAD the project — `StorageLoadException: ... 'Module.Name' is not a valid AttributeIdentifier` after a `create`/`change` with a `Module.Assoc = …` member, yet `mxcli exec` reported success | A one-qualifier member (`Module.Name`) that isn't a known association was written as an *attribute* ref, but a one-qualifier name can't be a valid attribute (attributes are bare or `Module.Entity.Attribute`) → unloadable .mpr. Usually the association's `create` failed earlier (non-idempotent) leaving it absent | `mdl/executor/cmd_microflows_builder_actions.go` (`resolveMemberChange`, the "Not an association in the authored module" branch) | When the domain model is available and the one-dot member isn't in `dm.Associations`/`dm.CrossAssociations`, `fb.addError` with an actionable "create the association first" message instead of writing an Attribute. Same-script associations are visible via `GetDomainModel`, so no false positive. Repro `mdl-examples/bug-tests/264-create-member-unknown-association.mdl`. FINDINGS #51 | | Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | | `grant view on page` / `grant execute on microflow\|nanoflow` / `grant access on odata\|published rest service` to a role from **another module** passes `mxcli check`/`exec` but fails the Mendix build with **CE0148 "reselect roles"** — the own-module role works | Document access (page/microflow/nanoflow/service `AllowedModuleRoles`) may only reference the document's **own** module roles; Studio Pro's picker only offers those. The grant path wrote `role.Module + "." + role.Name` verbatim with no same-module check (only `validateModuleRole` = role-exists-in-its-module), so a cross-module reference reached the model. The MOVE path already guarded this (`remapDocumentAccessRoles`) — GRANT didn't | `mdl/executor/cmd_security_defaults.go` (`checkDocumentAccessRolesSameModule`) + the 5 grant handlers in `mdl/executor/cmd_security_write.go` | Pre-check each grant: reject when any `role.Module != docModule` with an actionable message (name the doc's module + suggest the own-module role). Reject (don't silently remap) — a GRANT is explicit, so a wrong role/doc shouldn't be substituted. Wired into page/microflow/nanoflow/OData/published-REST grants. Repro `mdl-examples/bug-tests/ce0148-cross-module-grant.mdl` | diff --git a/mdl-examples/bug-tests/264-create-member-unknown-association.mdl b/mdl-examples/bug-tests/264-create-member-unknown-association.mdl new file mode 100644 index 000000000..14d057c73 --- /dev/null +++ b/mdl-examples/bug-tests/264-create-member-unknown-association.mdl @@ -0,0 +1,50 @@ +-- ============================================================================ +-- FINDINGS #51: create/change member referencing a non-existent association +-- ============================================================================ +-- +-- Symptom: a `create` (or `change`) activity that assigns a one-qualifier member +-- `Module.Name` which is NOT an existing association was written as an *attribute* +-- reference. A one-qualifier name can never be a valid attribute (attributes are +-- bare or `Module.Entity.Attribute`), so the result was an invalid AttributeIdentifier +-- and the next `mx check` could not even LOAD the project: +-- +-- Mendix.Modeler.Storage.StorageLoadException: ... 'Module.TimelinessBand_Period' +-- is not a valid AttributeIdentifier. +-- +-- This bit hard because it usually followed a `create association` that had failed +-- earlier in a script (non-idempotent, see #23/#51), leaving the association absent +-- while a later `create` still referenced it — a green exec into an unloadable .mpr. +-- +-- After fix: mxcli rejects the member at exec time with an actionable error naming +-- the missing association, instead of corrupting the .mpr. Associations created +-- earlier in the SAME script are still resolved correctly (no false positive). +-- +-- The commented-out ACT_Bad below is the repro (uncomment to see the rejection): +-- member "Demo.Missing_Assoc" on entity Demo.Child is not a known association ... +-- ACT_Good demonstrates the correct pattern (association created first). +-- ============================================================================ + +create or modify entity "Demo"."Parent" (Code: string(20)); +/ +create or modify entity "Demo"."Child" (Code: string(20)); +/ +create or modify association "Demo"."Child_Parent" +from "Demo"."Child" to "Demo"."Parent" +type reference; +/ + +-- Correct: the association exists (created above), so the member resolves. +create or modify microflow "Demo"."ACT_Good" ($P: Demo.Parent) +begin + $c = create "Demo"."Child" (Code = 'x', "Demo"."Child_Parent" = $P); + return; +end; +/ + +-- Repro (rejected at exec — uncomment to verify): +-- create or modify microflow "Demo"."ACT_Bad" () +-- begin +-- $c = create "Demo"."Child" (Code = 'x', "Demo"."Missing_Assoc" = empty); +-- return; +-- end; +-- / diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index a2e5348e0..d7cb7b391 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -1594,11 +1594,23 @@ func (fb *flowBuilder) resolveMemberChange(mc *microflows.MemberChange, memberNa return } } - // Not an association in the authored module — if the author - // qualified it (e.g. `Module.Attr`) the qualification is an - // error we must preserve rather than silently dropping; the - // writer will surface it during mx check. + // Not an association in the authored module. A single-qualifier + // name (`Module.Name`) can ONLY be an association — attributes are + // bare or `Module.Entity.Attribute` (two qualifiers). So a one-dot + // member that isn't a known association is a reference to an + // association that does not exist. Writing it as an Attribute + // produces an *unloadable* .mpr (StorageLoadException "... is not a + // valid AttributeIdentifier") rather than a clean CE error — reject + // it here instead (FINDINGS #51). Associations created earlier in the + // same script are visible via GetDomainModel, so this does not + // false-positive on same-script associations. + if strings.Count(memberName, ".") == 1 { + fb.addError("member %q on entity %s is not a known association (and a one-qualifier name cannot be an attribute) — create the association first (`create or modify association %s from ... to ...`) or fix the name", memberName, entityQN, memberName) + return + } if strings.Contains(memberName, ".") { + // Two-or-more-dot qualified attribute (Module.Entity.Attribute): + // preserve the authored qualification; mx check surfaces a wrong one. mc.AttributeQualifiedName = memberName } else if attrQN, ok := fb.resolveAttributeInEntityHierarchy(entityQN, memberName); ok { mc.AttributeQualifiedName = attrQN diff --git a/mdl/executor/cmd_microflows_member_identifier_test.go b/mdl/executor/cmd_microflows_member_identifier_test.go index 17489658c..1ea71bba5 100644 --- a/mdl/executor/cmd_microflows_member_identifier_test.go +++ b/mdl/executor/cmd_microflows_member_identifier_test.go @@ -95,3 +95,37 @@ func TestResolveMemberChange_UnquotedAssociationResolves(t *testing.T) { t.Errorf("unexpected errors: %v", fb.errors) } } + +// TestResolveMemberChange_RejectsUnknownAssociation guards FINDINGS #51: a +// one-qualifier member (`Module.Name`) that is not a known association cannot be an +// attribute either, so it must be rejected rather than serialized as an invalid +// Attribute (which yields an unloadable .mpr: "... is not a valid AttributeIdentifier"). +func TestResolveMemberChange_RejectsUnknownAssociation(t *testing.T) { + moduleID := model.ID("m") + backend := &mock.MockBackend{ + GetModuleByNameFunc: func(name string) (*model.Module, error) { + if name == "M" { + return &model.Module{BaseElement: model.BaseElement{ID: moduleID}, Name: name}, nil + } + return nil, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { + // Domain model exists but has NO associations — the referenced one is absent. + return &domainmodel.DomainModel{ContainerID: moduleID}, nil + }, + } + fb := &flowBuilder{backend: backend} + mc := µflows.MemberChange{} + fb.resolveMemberChange(mc, "M.Nonexistent_Assoc", "M.Child") + + if mc.AttributeQualifiedName != "" || mc.AssociationQualifiedName != "" { + t.Errorf("unknown association must not serialize: attr=%q assoc=%q", + mc.AttributeQualifiedName, mc.AssociationQualifiedName) + } + if len(fb.errors) == 0 { + t.Fatal("expected a validation error for the unknown association, got none") + } + if !strings.Contains(fb.errors[0], "not a known association") { + t.Errorf("unexpected error: %q", fb.errors[0]) + } +} From 97611a7832f3fb4518e8bb7f0cebdb771175021d Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 16:09:37 +0000 Subject: [PATCH 09/21] feat(check): flag duplicate widget names on a page (FINDINGS #15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mendix requires widget names to be unique per page and rejects duplicates with CE0495 "Duplicate name" — but `mxcli check --references` passed a page with, e.g., a container and a listview both named `ruTop`, and the failure only surfaced at MxBuild. Added checkDuplicateWidgetNames to the page context validator: it walks the widget tree, counts names, and reports each name used more than once (once per name, first-seen order). Runs at check time under --references, before the build. Verified end to end: a page with two `ruTop` widgets now reports the CE0495-class error; unique-named pages pass; full executor suite (no false positives on existing test pages). Unit + parsed-page tests added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- mdl/executor/validate_dup_widget_test.go | 47 ++++++++++++++++++++++++ mdl/executor/validate_page_context.go | 32 ++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 mdl/executor/validate_dup_widget_test.go diff --git a/mdl/executor/validate_dup_widget_test.go b/mdl/executor/validate_dup_widget_test.go new file mode 100644 index 000000000..1682a5516 --- /dev/null +++ b/mdl/executor/validate_dup_widget_test.go @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestCheckDuplicateWidgetNames_Unit is the pure-logic guard for FINDINGS #15. +func TestCheckDuplicateWidgetNames_Unit(t *testing.T) { + widgets := []*ast.WidgetV3{ + {Type: "container", Name: "ruTop", Children: []*ast.WidgetV3{ + {Type: "listview", Name: "ruTop"}, + }}, + } + errs := checkDuplicateWidgetNames(widgets) + if len(errs) != 1 || !strings.Contains(errs[0], "ruTop") { + t.Fatalf("expected one duplicate error for ruTop, got %v", errs) + } +} + +// TestCheckDuplicateWidgetNames_Parsed guards the end-to-end path: a page parsed +// from MDL where a container and a listview share a name must be flagged (CE0495). +func TestCheckDuplicateWidgetNames_Parsed(t *testing.T) { + src := `create or replace page "M"."P" (URL: 'p') { + container ruTop { + listview ruTop (DataSource: DATABASE M.Thing) { } + } +} +/` + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + pg, ok := prog.Statements[0].(*ast.CreatePageStmtV3) + if !ok { + t.Fatalf("statement 0 = %T, want *ast.CreatePageStmtV3", prog.Statements[0]) + } + dup := checkDuplicateWidgetNames(pg.Widgets) + if len(dup) != 1 || !strings.Contains(dup[0], "ruTop") { + t.Fatalf("expected duplicate ruTop error from parsed page, got %v (widget names may not be populated for containers)", dup) + } +} diff --git a/mdl/executor/validate_page_context.go b/mdl/executor/validate_page_context.go index 9d333c183..d5b7ffd93 100644 --- a/mdl/executor/validate_page_context.go +++ b/mdl/executor/validate_page_context.go @@ -29,10 +29,42 @@ func validatePageContextTree(params []ast.PageParameter, widgets []*ast.WidgetV3 // Walk the widget tree with context tracking var errors []string + errors = append(errors, checkDuplicateWidgetNames(widgets)...) walkWidgetsWithContext(widgets, paramNames, widgetNames, false, &errors) return errors } +// checkDuplicateWidgetNames flags any widget name that appears more than once on a +// page. Mendix requires widget names to be unique per page and rejects duplicates +// with CE0495 "Duplicate name" — which mxcli check otherwise passed (FINDINGS #15). +// Each duplicate name is reported once, in first-seen order. +func checkDuplicateWidgetNames(widgets []*ast.WidgetV3) []string { + counts := make(map[string]int) + var order []string + var walk func(ws []*ast.WidgetV3) + walk = func(ws []*ast.WidgetV3) { + for _, w := range ws { + if w.Name != "" { + if counts[w.Name] == 0 { + order = append(order, w.Name) + } + counts[w.Name]++ + } + walk(w.Children) + } + } + walk(widgets) + + var errors []string + for _, name := range order { + if counts[name] > 1 { + errors = append(errors, + fmt.Sprintf("duplicate widget name '%s' (used %d times) — Mendix requires unique widget names per page (CE0495)", name, counts[name])) + } + } + return errors +} + // collectWidgetNames recursively collects all widget names in the tree. func collectWidgetNames(widgets []*ast.WidgetV3, names map[string]bool) { for _, w := range widgets { From 08746d007842acb120bb6d8575aa1663837faf06 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 16:15:02 +0000 Subject: [PATCH 10/21] feat(check): flag unmapped workflow call-microflow parameters (FINDINGS #40) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow "call microflow" activity that did not map a required parameter of its target microflow passed both `mxcli check --references` and `mx check`, but Studio Pro rejects it (CE6677) and the workflow fails at the activity — the mapped and unmapped forms were indistinguishable to the checker. Added a reference-phase validator (runs under --references, where the target microflow is introspectable): for each workflow call-microflow, look up the target microflow's parameters via ListMicroflows and report any parameter not present in the activity's `with (...)` mappings. Microflows created in the same script are skipped (not yet queryable); a target not in the project is left to the missing-reference check. Wired as a new CreateWorkflowStmt case in validateWithContext. Verified end to end: an unmapped `Item` parameter is now reported; the mapped form passes; full executor suite green. Repro mdl-examples/bug-tests/265-workflow-unmapped-microflow-param.mdl. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .../265-workflow-unmapped-microflow-param.mdl | 35 ++++++++++ mdl/executor/validate.go | 8 +++ mdl/executor/validate_workflow_refs.go | 68 +++++++++++++++++++ 3 files changed, 111 insertions(+) create mode 100644 mdl-examples/bug-tests/265-workflow-unmapped-microflow-param.mdl create mode 100644 mdl/executor/validate_workflow_refs.go diff --git a/mdl-examples/bug-tests/265-workflow-unmapped-microflow-param.mdl b/mdl-examples/bug-tests/265-workflow-unmapped-microflow-param.mdl new file mode 100644 index 000000000..22cc6f32f --- /dev/null +++ b/mdl-examples/bug-tests/265-workflow-unmapped-microflow-param.mdl @@ -0,0 +1,35 @@ +-- ============================================================================ +-- FINDINGS #40: workflow call-microflow with an unmapped required parameter +-- ============================================================================ +-- +-- Symptom: a workflow "call microflow" activity that does NOT map a required +-- parameter of its target microflow passed both `mxcli check --references` and +-- `mx check`, but Studio Pro flags it (CE6677) and the workflow fails at the +-- activity. The mapped and unmapped forms were indistinguishable to the checker. +-- +-- After fix: `mxcli check --references` now reports each unmapped parameter: +-- call microflow 'Demo.ACT_Need': parameter 'Item' is not mapped — Mendix +-- requires every parameter of a workflow call-microflow to be mapped ... +-- +-- Requires a project (-p) — the check introspects the target microflow's params. +-- Setup (run first, against the project): +-- create or modify entity "Demo"."Thing" (Code: string(20)); +-- create or modify microflow "Demo"."ACT_Need" ($Item: Demo.Thing) begin return; end; +-- +-- WF_Ok maps the parameter (passes); WF_Bad omits it (flagged). +-- ============================================================================ + +create or modify workflow "Demo"."WF_Ok" + parameter $Context: Demo.Thing +begin + call microflow "Demo"."ACT_Need" with (Item = '$workflowContext'); +end workflow; +/ + +-- Unmapped required param — flagged by `check --references` (uncomment to verify): +-- create or modify workflow "Demo"."WF_Bad" +-- parameter $Context: Demo.Thing +-- begin +-- call microflow "Demo"."ACT_Need"; +-- end workflow; +-- / diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 999a2b363..5371d4b49 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -428,6 +428,14 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewValidationf("snippet '%s' has context errors:\n - %s", s.Name.String(), strings.Join(ctxErrors, "\n - ")) } + case *ast.CreateWorkflowStmt: + // Reference check: every workflow call-microflow must map all of its + // target microflow's parameters (FINDINGS #40). Syntax-only workflow checks + // (MDL-WF01/02/03) run separately in the no-project phase. + if refErrors := validateWorkflowParameterMappings(ctx, s, sc); len(refErrors) > 0 { + return mdlerrors.NewValidationf("workflow '%s' has reference errors:\n - %s", + s.Name.String(), strings.Join(refErrors, "\n - ")) + } case *ast.CreateViewEntityStmt: if s.Name.Module != "" && !sc.modules[s.Name.Module] { if _, err := findModule(ctx, s.Name.Module); err != nil { diff --git a/mdl/executor/validate_workflow_refs.go b/mdl/executor/validate_workflow_refs.go new file mode 100644 index 000000000..8a5d02dc0 --- /dev/null +++ b/mdl/executor/validate_workflow_refs.go @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Reference-based (project-connected) validation for workflows. Runs under +// `check --references`, where the target microflows can be introspected. See +// validate_workflow.go for the syntax-only (no-project) checks. +package executor + +import ( + "fmt" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// validateWorkflowParameterMappings checks that each workflow "call microflow" +// activity maps every parameter of its target microflow. Mendix rejects an +// unmapped parameter (CE6677 — "should accept parameters ...") and the workflow +// would fail at the activity, but mxcli check passed it (FINDINGS #40). Microflows +// created in the same script are skipped (not yet queryable); a target microflow +// that isn't in the project is left to the missing-reference check. +func validateWorkflowParameterMappings(ctx *ExecContext, s *ast.CreateWorkflowStmt, sc *scriptContext) []string { + if ctx == nil || ctx.Backend == nil { + return nil + } + h, err := getHierarchy(ctx) + if err != nil { + return nil + } + mfs, err := ctx.Backend.ListMicroflows() + if err != nil { + return nil + } + paramsByMF := make(map[string][]string, len(mfs)) + for _, mf := range mfs { + names := make([]string, 0, len(mf.Parameters)) + for _, p := range mf.Parameters { + names = append(names, p.Name) + } + paramsByMF[h.GetQualifiedName(mf.ContainerID, mf.Name)] = names + } + + var errs []string + walkWorkflowActivities(s.Activities, func(act ast.WorkflowActivityNode) { + cm, ok := act.(*ast.WorkflowCallMicroflowNode) + if !ok { + return + } + mfQN := cm.Microflow.String() + if sc != nil && sc.microflows[mfQN] { + return // created in the same script — cannot introspect its parameters + } + want, known := paramsByMF[mfQN] + if !known { + return // target microflow not in project; the missing-ref check covers it + } + mapped := make(map[string]bool, len(cm.ParameterMappings)) + for _, pm := range cm.ParameterMappings { + mapped[pm.Parameter] = true + } + for _, name := range want { + if !mapped[name] { + errs = append(errs, fmt.Sprintf( + "call microflow '%s': parameter '%s' is not mapped — Mendix requires every parameter of a workflow call-microflow to be mapped (add `with (%s = ...)`)", + mfQN, name, name)) + } + } + }) + return errs +} From 68acf0e384a8fff376ecb584185c0213fcfc1158 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 16:18:00 +0000 Subject: [PATCH 11/21] feat(check): flag aggregate/unknown functions in create/change attribute values (FINDINGS #17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An aggregate (sum/count/average/…) or unknown function used inside a `create` or `change` attribute value fails the build with CE0117, but `mxcli check` only inspected return/if/declare/set expressions — the same MDL044 check never reached attribute-assignment values, so `RowTotal = formatDecimal(sum($cells), '0.00')` passed check and failed MxBuild with a message that didn't even mention `sum`. Wired checkExprFunctions (MDL044) into the CreateObjectStmt and ChangeObjectStmt cases of the microflow body walk. Verified: `sum()` in a create attribute now reports MDL044 with the "assign the aggregate to a variable first" hint; legit functions (formatDecimal/trim) pass; full executor suite green. Test cases added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- mdl/executor/validate_microflow.go | 11 +++++++++++ mdl/executor/validate_microflow_expr_test.go | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index a4a52b1f6..ca6ac05a6 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -317,6 +317,17 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { v.loopDepth++ v.walkBody(stmt.Body) v.loopDepth-- + case *ast.CreateObjectStmt: + // Attribute values in a `create` are expressions too — an aggregate + // (sum/count/…) or an unknown function here fails the build with CE0117, + // but check previously only inspected return/if/declare/set (FINDINGS #17). + for _, ch := range stmt.Changes { + v.checkExprFunctions(fmt.Sprintf("create %s attribute '%s'", stmt.EntityType.String(), ch.Attribute), ch.Value) + } + case *ast.ChangeObjectStmt: + for _, ch := range stmt.Changes { + v.checkExprFunctions(fmt.Sprintf("change '%s' attribute '%s'", stmt.Variable, ch.Attribute), ch.Value) + } } // Check error handling inside loops if eh := stmtErrorHandling(s); eh != nil { diff --git a/mdl/executor/validate_microflow_expr_test.go b/mdl/executor/validate_microflow_expr_test.go index 706845bbc..517e8bced 100644 --- a/mdl/executor/validate_microflow_expr_test.go +++ b/mdl/executor/validate_microflow_expr_test.go @@ -46,6 +46,10 @@ func TestValidateMicroflow_UnknownFunction(t *testing.T) { // fires, but the hint must steer to "assign to a variable first", not a // did-you-mean against an unrelated math function (finding #7). {"count aggregate hint", "declare $ok Boolean = if count($x) > 0 then true else false;", true, "aggregate activity"}, + // FINDINGS #17: an aggregate inside a `create` attribute value must also be + // flagged — previously only return/if/declare/set expressions were checked. + {"aggregate in create attr", `$r = create "M"."E" (Total = formatDecimal(sum($x), '0.00'));`, true, "aggregate activity"}, + {"known func in create attr", `$r = create "M"."E" (Total = trim($x));`, false, ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From 1b390dce3ba78dd22f0190856363922d995dd502 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 17:30:49 +0000 Subject: [PATCH 12/21] fix(workflows): match call-microflow outcomes to return type + normalize context var (FINDINGS #39 regression) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 11.9 storage-name fix (CallMicroflowTask→CallMicroflowActivity) let the runtime load the model again, but the retest found the new class also structures outcomes and the parameter-mapping expression differently — so a call-microflow with a parameter or a non-void return now failed `mx check` with two errors that the tolerant old class had passed: - CE6686 "outcomes do not match the configured microflow": autoBindCallMicroflow injected a single VoidConditionOutcome regardless of return type. The 11.9+ class requires outcomes that match the microflow — a Boolean return needs true/false BooleanConditionOutcomes. defaultCallMicroflowOutcomes now generates them from the target microflow's ReturnType (Boolean → two branches, else → single default). - CE0117 "Error(s) in expression": the context parameter is named "WorkflowContext" and 11.9+ expressions are case-sensitive, so a user-written `$workflowContext` (the form in every example) is an undefined variable. normalizeWorkflowContextExpr rewrites it to `$WorkflowContext`. Verified against real mxbuild on BOTH classes: - Mendix 11.12.1 (CallMicroflowActivity): `call microflow ACT with (Ctx='$workflowContext')` on a Boolean-returning microflow → 0 errors (was 2). - Mendix 11.6.3 (CallMicroflowTask): same script → 0 errors (no regression). Unit tests for both helpers; bug-test 263 updated to the param+Boolean-outcome case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- ...63-workflow-callmicroflow-storage-name.mdl | 25 ++++- .../cmd_workflows_callmicroflow_test.go | 64 +++++++++++ mdl/executor/cmd_workflows_write.go | 103 +++++++++++------- 3 files changed, 149 insertions(+), 43 deletions(-) create mode 100644 mdl/executor/cmd_workflows_callmicroflow_test.go diff --git a/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl b/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl index 1ec7909b6..ef9131fb8 100644 --- a/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl +++ b/mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl @@ -23,22 +23,37 @@ -- HasOwner→HasOwnerAttr 11.9 gate). The semantic model is unchanged; only the -- emitted $Type differs. -- --- Verify (on an 11.9+ project, e.g. Mendix 11.12.1): +-- Follow-up regression (retest of the storage-name fix): correcting the $Type to +-- CallMicroflowActivity exposed that the 11.9+ class also structures OUTCOMES and +-- the parameter-mapping expression differently from the old class: +-- * CE6686 — outcomes must MATCH the microflow's return type. A Boolean-returning +-- microflow needs true/false BooleanConditionOutcomes, not a lone Void outcome. +-- * CE0117 — the context variable is case-sensitive on 11.9+. The context +-- parameter is named "WorkflowContext", so a user-written `$workflowContext` +-- (lowercase) is undefined. mxcli now normalizes it to `$WorkflowContext`. +-- +-- Verify (on an 11.9+ project, e.g. Mendix 11.12.1) AND on a pre-11.9 project — both +-- must build 0 errors: -- mxcli exec 263-workflow-callmicroflow-storage-name.mdl -p App.mpr -- <mxbuild>/modeler/mx check App.mpr -> 0 errors -- mxcli run --local -p App.mpr --ensure-db -> model loads (previously died) -- ============================================================================ -create or modify microflow "MyFirstModule"."ACT_ApproveStep" () +create or modify entity "MyFirstModule"."Ctx" (WeekNumber: integer); +/ + +-- Boolean-returning microflow with an entity parameter — exercises both CE6686 +-- (return-type-matched outcomes) and CE0117 (context-variable case). +create or modify microflow "MyFirstModule"."ACT_ApproveStep" ($Ctx: MyFirstModule.Ctx) +returns boolean as $ok begin - return; + return true; end; / create or modify workflow "MyFirstModule"."WF_CallMicroflowStorageName" parameter $Context: MyFirstModule.Ctx begin - call microflow "MyFirstModule"."ACT_ApproveStep" - comment 'automated approve step'; + call microflow "MyFirstModule"."ACT_ApproveStep" with (Ctx = '$workflowContext'); end workflow; / diff --git a/mdl/executor/cmd_workflows_callmicroflow_test.go b/mdl/executor/cmd_workflows_callmicroflow_test.go new file mode 100644 index 000000000..6748cc85c --- /dev/null +++ b/mdl/executor/cmd_workflows_callmicroflow_test.go @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/workflows" +) + +// TestNormalizeWorkflowContextExpr guards the CE0117 half of the FINDINGS #39 +// regression: the 11.9+ CallMicroflowActivity is case-sensitive, so a user-written +// `$workflowContext` must be normalized to the context parameter name +// `$WorkflowContext`. +func TestNormalizeWorkflowContextExpr(t *testing.T) { + cases := map[string]string{ + "$workflowContext": "$WorkflowContext", + "$WORKFLOWCONTEXT": "$WorkflowContext", + "$WorkflowContext": "$WorkflowContext", + "$workflowContext/Field": "$WorkflowContext/Field", + "$Other": "$Other", + "'literal'": "'literal'", + } + for in, want := range cases { + if got := normalizeWorkflowContextExpr(in); got != want { + t.Errorf("normalizeWorkflowContextExpr(%q) = %q, want %q", in, got, want) + } + } +} + +// TestDefaultCallMicroflowOutcomes guards the CE6686 half of the FINDINGS #39 +// regression: default outcomes must match the target microflow's return type — +// Boolean → two BooleanConditionOutcomes, anything else → a single default. +func TestDefaultCallMicroflowOutcomes(t *testing.T) { + boolMF := µflows.Microflow{Name: "ACT", ReturnType: microflows.BooleanType{}} + outs := defaultCallMicroflowOutcomes(boolMF) + if len(outs) != 2 { + t.Fatalf("boolean return: got %d outcomes, want 2 (true/false)", len(outs)) + } + sawTrue, sawFalse := false, false + for _, o := range outs { + b, ok := o.(*workflows.BooleanConditionOutcome) + if !ok { + t.Fatalf("boolean return: outcome %T, want *BooleanConditionOutcome", o) + } + sawTrue = sawTrue || b.Value + sawFalse = sawFalse || !b.Value + } + if !sawTrue || !sawFalse { + t.Errorf("boolean return: want both true and false outcomes, got true=%v false=%v", sawTrue, sawFalse) + } + + // Void / nil / non-branching → single VoidConditionOutcome. + for _, mf := range []*microflows.Microflow{nil, {ReturnType: microflows.VoidType{}}, {ReturnType: microflows.StringType{}}} { + outs := defaultCallMicroflowOutcomes(mf) + if len(outs) != 1 { + t.Fatalf("non-boolean return %v: got %d outcomes, want 1", mf, len(outs)) + } + if _, ok := outs[0].(*workflows.VoidConditionOutcome); !ok { + t.Errorf("non-boolean return: outcome %T, want *VoidConditionOutcome", outs[0]) + } + } +} diff --git a/mdl/executor/cmd_workflows_write.go b/mdl/executor/cmd_workflows_write.go index 176eeea59..1cad00368 100644 --- a/mdl/executor/cmd_workflows_write.go +++ b/mdl/executor/cmd_workflows_write.go @@ -5,6 +5,7 @@ package executor import ( "fmt" + "regexp" "strings" "unicode" @@ -12,6 +13,7 @@ import ( mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" "github.com/mendixlabs/mxcli/sdk/workflows" ) @@ -676,62 +678,87 @@ func autoBindActivitiesInFlow(ctx *ExecContext, activities []workflows.WorkflowA } // autoBindCallMicroflow resolves microflow parameters and auto-generates ParameterMappings. -// Also ensures a default VoidConditionOutcome exists (required by Mendix runtime — CE6686). +// Ensures default outcomes that MATCH the target microflow's return type, which the +// Mendix 11.9+ CallMicroflowActivity requires (CE6686: "outcomes do not match the +// configured microflow"). The pre-11.9 CallMicroflowTask tolerated a lone +// VoidConditionOutcome regardless of return type, which is why this used to be +// hardcoded to Void (FINDINGS #39 regression). func autoBindCallMicroflow(ctx *ExecContext, task *workflows.CallMicroflowTask) { // Sanitize name task.Name = sanitizeActivityName(task.Name) - // Auto-generate Default outcome if no outcomes specified. - // This must run regardless of whether parameter mappings exist, - // because the Mendix runtime requires a VoidConditionOutcome on - // every CallMicroflowTask (CE6686: "outcomes do not match"). - if len(task.Outcomes) == 0 { - outcome := &workflows.VoidConditionOutcome{ - Flow: &workflows.Flow{}, + // Normalize the workflow-context variable in explicit parameter mappings to the + // actual context parameter name ("WorkflowContext"). Mendix expressions are + // case-sensitive on 11.9+, so a user-written `$workflowContext` is an undefined + // variable → CE0117 (FINDINGS #39 regression). The pre-11.9 class did not flag it. + for _, pm := range task.ParameterMappings { + pm.Expression = normalizeWorkflowContextExpr(pm.Expression) + } + + // Look up the target microflow — needed both for return-type-matched outcomes + // and for parameter auto-binding. + var targetMF *microflows.Microflow + if mfs, err := ctx.Backend.ListMicroflows(); err == nil { + if h, err := getHierarchy(ctx); err == nil { + for _, mf := range mfs { + if h.GetModuleName(h.FindModuleID(mf.ContainerID))+"."+mf.Name == task.Microflow { + targetMF = mf + break + } + } } - outcome.BaseElement.ID = model.ID(types.GenerateID()) - outcome.Flow.BaseElement.ID = model.ID(types.GenerateID()) - task.Outcomes = append(task.Outcomes, outcome) - } - - // Skip parameter auto-binding if already has explicit mappings - if len(task.ParameterMappings) > 0 { - return } - // Look up the microflow to get its parameters - mfs, err := ctx.Backend.ListMicroflows() - if err != nil { - return - } - - h, err := getHierarchy(ctx) - if err != nil { - return + // Auto-generate outcomes matching the microflow's return type when none given. + if len(task.Outcomes) == 0 { + task.Outcomes = defaultCallMicroflowOutcomes(targetMF) } - for _, mf := range mfs { - modID := h.FindModuleID(mf.ContainerID) - modName := h.GetModuleName(modID) - qualifiedName := modName + "." + mf.Name - if qualifiedName != task.Microflow { - continue - } - - // Found the microflow — bind parameters - for _, param := range mf.Parameters { - paramQualifiedName := qualifiedName + "." + param.Name + // Auto-bind parameters (context) if no explicit mappings were given. + if len(task.ParameterMappings) == 0 && targetMF != nil { + for _, param := range targetMF.Parameters { mapping := &workflows.ParameterMapping{ - Parameter: paramQualifiedName, + Parameter: task.Microflow + "." + param.Name, Expression: "$WorkflowContext", } mapping.BaseElement.ID = model.ID(types.GenerateID()) task.ParameterMappings = append(task.ParameterMappings, mapping) } - break } } +// defaultCallMicroflowOutcomes builds the default outcome set for a call-microflow +// activity based on the target microflow's return type: Boolean → true/false +// branches; anything else (void, entity, string, …) → a single default outcome. +// Enumeration returns would need one outcome per value; until that is wired, they +// fall through to the single-default form (still a Void outcome). +func defaultCallMicroflowOutcomes(mf *microflows.Microflow) []workflows.ConditionOutcome { + newFlow := func() *workflows.Flow { + f := &workflows.Flow{} + f.BaseElement.ID = model.ID(types.GenerateID()) + return f + } + if mf != nil && mf.ReturnType != nil && mf.ReturnType.GetTypeName() == "Boolean" { + yes := &workflows.BooleanConditionOutcome{Value: true, Flow: newFlow()} + yes.BaseElement.ID = model.ID(types.GenerateID()) + no := &workflows.BooleanConditionOutcome{Value: false, Flow: newFlow()} + no.BaseElement.ID = model.ID(types.GenerateID()) + return []workflows.ConditionOutcome{yes, no} + } + o := &workflows.VoidConditionOutcome{Flow: newFlow()} + o.BaseElement.ID = model.ID(types.GenerateID()) + return []workflows.ConditionOutcome{o} +} + +// normalizeWorkflowContextExpr rewrites a case-insensitive `$workflowContext` +// reference to the exact context parameter name `$WorkflowContext`. In a workflow +// the only in-scope variable is the context, so this is unambiguous. +func normalizeWorkflowContextExpr(expr string) string { + return workflowContextRe.ReplaceAllString(expr, "$$WorkflowContext") +} + +var workflowContextRe = regexp.MustCompile(`(?i)\$workflowcontext`) + // autoBindCallWorkflow resolves workflow parameters and generates ParameterMappings. func autoBindCallWorkflow(ctx *ExecContext, act *workflows.CallWorkflowActivity) { // Sanitize name From c91f5181e1200642fea06133ffbc5c08e59ef96f Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 18:23:03 +0000 Subject: [PATCH 13/21] fix(pages): reject a page with widgets but no Layout instead of silently dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `create page X { ...widgets... }` with no `Layout:` clause reported "Created page" but produced an EMPTY page: buildPageV3 only builds a LayoutCall when Layout: is present, and the widget tree is built into that LayoutCall's placeholder arguments — so with no LayoutCall the widgets have nowhere to attach and were silently dropped. Mendix then rejects the layout-less page at build with CE1613 ("layout 'dummyModule.dummyName' no longer exists" — its internal placeholder for a missing layout). mxcli check passed; the widgets were simply gone — the same silent data-loss pattern the TimeRegistration findings are about. buildPageV3 now returns an actionable error when a page has body widgets (or placeholder blocks) but no LayoutCall, distinguishing "no Layout: clause" from "layout not found". Empty layout-less pages are unaffected (nothing to drop), and snippets (buildSnippetV3) are layout-less by design and untouched. Discovered while reproducing FINDINGS #49 (which no longer reproduces) against a real Mendix 11.12.1 project + mx check. Unit tests (widgets→error, empty→ok); repro mdl-examples/bug-tests/266; fixed the pre-existing no-layout page in the ce0148 example. Symptom row added to fix-issue.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .../266-page-without-layout-drops-widgets.mdl | 34 +++++++++++++++ .../bug-tests/ce0148-cross-module-grant.mdl | 3 +- mdl/executor/cmd_pages_builder_v3.go | 17 ++++++++ mdl/executor/cmd_pages_nolayout_test.go | 41 +++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl create mode 100644 mdl/executor/cmd_pages_nolayout_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 35c8b378d..d351f945e 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -23,6 +23,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | `describe` shows `$var = list operation %T;` (with type name) | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatListOperation()` | Add `case *microflows.XxxOperation:` before the `default` | | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | +| A `create page` reported success but the built page is EMPTY — every widget gone — and `mx check` fails with CE1613 "The selected layout 'dummyModule.dummyName' no longer exists" | The page had no `Layout:` clause, so `buildPageV3` created no `LayoutCall`; the widget tree is built into the LayoutCall's placeholder arguments, so with no LayoutCall the widgets have nowhere to attach and are silently dropped. `dummyModule.dummyName` is *Mendix's* placeholder for a missing layout, not something mxcli writes | `mdl/executor/cmd_pages_builder_v3.go` (`buildPageV3`, the `if page.LayoutCall != nil` block) | Reject a page that has body widgets (or placeholder blocks) but no LayoutCall — distinguish "no Layout: clause" from "layout not found" in the message. A Mendix page always needs a layout; snippets (buildSnippetV3) are layout-less and unaffected. Repro `mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl` | | `mx check` fails to LOAD the project — `StorageLoadException: ... 'Module.Name' is not a valid AttributeIdentifier` after a `create`/`change` with a `Module.Assoc = …` member, yet `mxcli exec` reported success | A one-qualifier member (`Module.Name`) that isn't a known association was written as an *attribute* ref, but a one-qualifier name can't be a valid attribute (attributes are bare or `Module.Entity.Attribute`) → unloadable .mpr. Usually the association's `create` failed earlier (non-idempotent) leaving it absent | `mdl/executor/cmd_microflows_builder_actions.go` (`resolveMemberChange`, the "Not an association in the authored module" branch) | When the domain model is available and the one-dot member isn't in `dm.Associations`/`dm.CrossAssociations`, `fb.addError` with an actionable "create the association first" message instead of writing an Attribute. Same-script associations are visible via `GetDomainModel`, so no false positive. Repro `mdl-examples/bug-tests/264-create-member-unknown-association.mdl`. FINDINGS #51 | | Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | diff --git a/mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl b/mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl new file mode 100644 index 000000000..d57b9305a --- /dev/null +++ b/mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl @@ -0,0 +1,34 @@ +-- ============================================================================ +-- Follow-up finding: a page created WITHOUT a `Layout:` clause silently drops +-- all its widgets +-- ============================================================================ +-- +-- Symptom: `create page X { ...widgets... }` with no `Layout:` clause reported +-- "Created page" (only a log warning), but the page had NO layout call — so the +-- widget tree had nowhere to attach and was silently dropped. Mendix then rejects +-- the layout-less page at build with CE1613 ("The selected layout +-- 'dummyModule.dummyName' no longer exists" — its internal placeholder for a +-- missing layout). `mxcli check` passed; the widgets were simply gone. +-- +-- A Mendix page always needs a layout; its widgets are placed into the layout's +-- placeholders. Without one there is no valid page. After fix, mxcli rejects the +-- build with an actionable error instead of producing a broken, widget-less page. +-- +-- The BAD form (rejected at exec — uncomment to see the error): +-- create or replace page "MyFirstModule"."NoLayout" (URL: 'nl') { +-- container c1 { dynamictext dt (Content: 'x') } +-- } +-- -> page 'MyFirstModule.NoLayout' has widgets but no Layout: clause — ... +-- +-- The correct form specifies a layout, so the widgets are placed: +-- ============================================================================ + +create or replace page "MyFirstModule"."WithLayout" ( + URL: 'withlayout', + Layout: Atlas_Core.Atlas_Default +) { + container c1 { + dynamictext dt (Content: 'Hello') + } +} +/ diff --git a/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl b/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl index daf2c3333..2f3a4a832 100644 --- a/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl +++ b/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl @@ -24,7 +24,8 @@ create module role Alpha.User description 'Alpha module user'; create module role Beta.Manager description 'Beta module manager'; create page Alpha.Overview ( - title: 'Overview' + title: 'Overview', + layout: Atlas_Core.Atlas_Default ) { layoutgrid g1 { row r1 { diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index a16736116..450cfa16b 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -150,6 +150,23 @@ func (pb *pageBuilder) buildPageV3(s *ast.CreatePageStmtV3) (*pages.Page, error) pb.localVariables[v.Name] = true } + // A page's widget tree is built into the LayoutCall's placeholder arguments + // below — so without a LayoutCall the widgets have nowhere to go and are + // silently dropped, and Mendix rejects the layout-less page at build time + // (CE1613, "layout … no longer exists"). This happens when the `Layout:` clause + // is omitted, or names a layout that does not exist. Reject it with an + // actionable error instead of producing a broken page that lost its widgets. + if page.LayoutCall == nil && (len(s.Widgets) > 0 || len(s.Placeholders) > 0) { + if s.Layout == "" { + return nil, mdlerrors.NewValidationf( + "page '%s' has widgets but no Layout: clause — a Mendix page requires a layout to place its widgets (without one they are dropped and the build fails CE1613). Add a layout, e.g. `Layout: Atlas_Core.Atlas_Default`.", + s.Name.String()) + } + return nil, mdlerrors.NewValidationf( + "page '%s' references layout '%s', which was not found — its widgets would be dropped. Use an existing layout (list them with `show catalog table layouts`).", + s.Name.String(), s.Layout) + } + // Build one FormCallArgument per layout placeholder (issue #532). Bare body // widgets bind to Main; a `placeholder <Name> { … }` block binds to that // named placeholder. The Main argument is always emitted (possibly empty) to diff --git a/mdl/executor/cmd_pages_nolayout_test.go b/mdl/executor/cmd_pages_nolayout_test.go new file mode 100644 index 000000000..1b0dd7a66 --- /dev/null +++ b/mdl/executor/cmd_pages_nolayout_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// TestBuildPageV3_WidgetsWithoutLayout guards the layout-drop bug: a page with body +// widgets but no Layout: clause has no LayoutCall to place them into, so the widgets +// were silently dropped and Mendix rejected the page (CE1613). It must now be a clear +// error instead. +func TestBuildPageV3_WidgetsWithoutLayout(t *testing.T) { + s := &ast.CreatePageStmtV3{ + Name: ast.QualifiedName{Module: "M", Name: "NoLayout"}, + Widgets: []*ast.WidgetV3{ + {Type: "container", Name: "c1", Children: []*ast.WidgetV3{ + {Type: "dynamictext", Name: "dt", Properties: map[string]any{"Content": "x"}}, + }}, + }, + } + _, err := newPopupPageBuilder().buildPageV3(s) + if err == nil { + t.Fatal("expected an error for a page with widgets but no Layout:, got nil (widgets would be dropped)") + } + if !strings.Contains(err.Error(), "no Layout") { + t.Errorf("error = %q, want it to mention the missing Layout: clause", err.Error()) + } +} + +// TestBuildPageV3_NoWidgetsNoLayout confirms the guard does NOT fire for an empty +// page (no widgets to drop) — matching the existing popup-defaults test behavior. +func TestBuildPageV3_NoWidgetsNoLayout(t *testing.T) { + s := &ast.CreatePageStmtV3{Name: ast.QualifiedName{Module: "M", Name: "Empty"}} + if _, err := newPopupPageBuilder().buildPageV3(s); err != nil { + t.Errorf("empty layout-less page should build without error, got %v", err) + } +} From b6a5043a60768f244fbccc2664688a6de80a9185 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 29 Jul 2026 19:07:17 +0000 Subject: [PATCH 14/21] fix(workflows): parse CallMicroflowActivity storage name on read (11.9+ roundtrip) The 11.9+ write path emits `Workflows$CallMicroflowActivity`, but the legacy BSON parser only registered `Workflows$CallMicroflowTask`. On an 11.9+ project a written call-microflow activity read back as an unknown type, so DESCRIBE emitted the `-- [Workflows$CallMicroflowActivity] ...` fallback comment instead of `call microflow ... with (...)`. Register the new $Type against the same parser (it already reads Outcomes + ParameterMappings) so the activity round-trips on both pre- and post-11.9 projects. Fixes the integration failures TestRoundtripWorkflow_Comprehensive and TestRoundtripWorkflow_CallMicroflowWithParams on the 11.9.0 CI runner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- sdk/mpr/parser_workflow.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/mpr/parser_workflow.go b/sdk/mpr/parser_workflow.go index 63d918920..285bec3ac 100644 --- a/sdk/mpr/parser_workflow.go +++ b/sdk/mpr/parser_workflow.go @@ -183,6 +183,7 @@ func init() { "Workflows$SingleUserTaskActivity": func(r map[string]any) workflows.WorkflowActivity { return parseUserTask(r) }, "Workflows$MultiUserTaskActivity": func(r map[string]any) workflows.WorkflowActivity { return parseMultiUserTask(r) }, "Workflows$CallMicroflowTask": func(r map[string]any) workflows.WorkflowActivity { return parseCallMicroflowTask(r) }, + "Workflows$CallMicroflowActivity": func(r map[string]any) workflows.WorkflowActivity { return parseCallMicroflowTask(r) }, "Workflows$CallWorkflowActivity": func(r map[string]any) workflows.WorkflowActivity { return parseCallWorkflowActivity(r) }, "Workflows$ExclusiveSplitActivity": func(r map[string]any) workflows.WorkflowActivity { return parseExclusiveSplitActivity(r) }, "Workflows$ParallelSplitActivity": func(r map[string]any) workflows.WorkflowActivity { return parseParallelSplitActivity(r) }, From c663f7afa902761f0e4a2d3be74aed946ac43960 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 03:18:00 +0000 Subject: [PATCH 15/21] fix(alter-page): resolve association-source entity for INSERT/REPLACE bindings (FINDINGS #55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALTER PAGE INSERT/REPLACE into a ListView bound `from association` produced a widget whose Attribute binding pointed at the wrong entity — the outer data view's entity instead of the association's destination — or no binding at all. `mxcli check` passed; the Mendix build then failed with CE1613 "The selected attribute 'Module.OuterEntity.Attr' no longer exists", and DESCRIBE masked it by printing only the short attribute name. Root cause: the page mutator read the enclosing entity from DataSource.EntityRef.Entity, which is only populated for a DIRECT entity ref (database source). An AssociationSource stores its destination on the last DomainModels$EntityRefStep of an IndirectEntityRef, so the mutator saw no entity for the list and left the context at the outer data view's entity; the inserted bare attribute then resolved against that outer entity. Fix: extractEntityFromDataSource now also reads the IndirectEntityRef's last EntityRefStep.DestinationEntity (new lastStepDestinationEntity helper), so a list bound `from association` reports its correct child entity to INSERT/REPLACE. Verified on real mxbuild 11.12.1: the nested dataview→association-listview→insert and →replace cases now `mx check` with 0 errors (CE1613 before). Unit guard TestEnclosingEntity_AssociationSource; repro mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .../55-alter-page-insert-assoc-binding.mdl | 58 ++++++++++++++++ mdl/backend/pagemutator/mutator.go | 30 ++++++++ mdl/backend/pagemutator/mutator_test.go | 68 +++++++++++++++++++ 4 files changed, 157 insertions(+) create mode 100644 mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index d351f945e..cbb988557 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -24,6 +24,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | | A `create page` reported success but the built page is EMPTY — every widget gone — and `mx check` fails with CE1613 "The selected layout 'dummyModule.dummyName' no longer exists" | The page had no `Layout:` clause, so `buildPageV3` created no `LayoutCall`; the widget tree is built into the LayoutCall's placeholder arguments, so with no LayoutCall the widgets have nowhere to attach and are silently dropped. `dummyModule.dummyName` is *Mendix's* placeholder for a missing layout, not something mxcli writes | `mdl/executor/cmd_pages_builder_v3.go` (`buildPageV3`, the `if page.LayoutCall != nil` block) | Reject a page that has body widgets (or placeholder blocks) but no LayoutCall — distinguish "no Layout: clause" from "layout not found" in the message. A Mendix page always needs a layout; snippets (buildSnippetV3) are layout-less and unaffected. Repro `mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl` | +| `ALTER PAGE INSERT`/`REPLACE` into a list bound `from association` produces a widget whose Attribute binds to the WRONG entity (the outer data view's) or nothing — `mxcli check` ✓ but `mx check` fails **CE1613** "The selected attribute 'Module.OuterEntity.Attr' no longer exists"; DESCRIBE masks it by printing only the short attribute name | The ALTER mutator reads the enclosing entity from `DataSource.EntityRef.Entity`, which is only set for a DIRECT ref (database source). An `AssociationSource` stores its destination on the last `DomainModels$EntityRefStep` of an `IndirectEntityRef`, so the list reported no entity and the context stayed at the outer data view — the bare inserted attribute then resolved against that outer entity | `mdl/backend/pagemutator/mutator.go` (`extractEntityFromDataSource` / `lastStepDestinationEntity`) | Also read the `IndirectEntityRef`'s last `EntityRefStep.DestinationEntity` so a `from association` list reports its child entity to INSERT/REPLACE. Unit guard `TestEnclosingEntity_AssociationSource`; repro `mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl`. FINDINGS #55 | | `mx check` fails to LOAD the project — `StorageLoadException: ... 'Module.Name' is not a valid AttributeIdentifier` after a `create`/`change` with a `Module.Assoc = …` member, yet `mxcli exec` reported success | A one-qualifier member (`Module.Name`) that isn't a known association was written as an *attribute* ref, but a one-qualifier name can't be a valid attribute (attributes are bare or `Module.Entity.Attribute`) → unloadable .mpr. Usually the association's `create` failed earlier (non-idempotent) leaving it absent | `mdl/executor/cmd_microflows_builder_actions.go` (`resolveMemberChange`, the "Not an association in the authored module" branch) | When the domain model is available and the one-dot member isn't in `dm.Associations`/`dm.CrossAssociations`, `fb.addError` with an actionable "create the association first" message instead of writing an Attribute. Same-script associations are visible via `GetDomainModel`, so no false positive. Repro `mdl-examples/bug-tests/264-create-member-unknown-association.mdl`. FINDINGS #51 | | Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | diff --git a/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl b/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl new file mode 100644 index 000000000..e449f30e5 --- /dev/null +++ b/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl @@ -0,0 +1,58 @@ +-- ============================================================================ +-- FINDINGS #55: ALTER PAGE INSERT/REPLACE drops the attribute binding when the +-- target lives inside a list bound `from association` +-- ============================================================================ +-- +-- Symptom: inserting (or replacing) a data-bound widget into a ListView whose +-- datasource is an association (`from association`, i.e. a nested master-detail +-- list) produced a widget whose Attribute binding pointed at the WRONG entity — +-- the OUTER data view's entity instead of the association's destination entity — +-- or no binding at all. `mxcli check` passed; the Mendix build then failed with +-- [CE1613] "The selected attribute 'Module.OuterEntity.Attr' no longer exists." +-- and DESCRIBE masked it by printing only the short attribute name. +-- +-- Root cause: the ALTER PAGE mutator reads the enclosing entity from +-- DataSource.EntityRef.Entity. That field is only populated for a DIRECT entity +-- ref (database source). An AssociationSource stores its destination on the last +-- DomainModels$EntityRefStep of an IndirectEntityRef, so the mutator saw no +-- entity for the list and left the context at the outer data view's entity. The +-- inserted bare attribute then resolved against that outer entity. +-- +-- Fix: extractEntityFromDataSource (mdl/backend/pagemutator/mutator.go) now also +-- reads the IndirectEntityRef's last EntityRefStep.DestinationEntity, so a list +-- bound `from association` reports the correct child entity to INSERT/REPLACE. +-- +-- Verify (Mendix 11.x): exec this script, then `mx check App.mpr` → 0 errors. +-- The probe dynamictext must bind to WeekRow.RowTotal, not Week.RowTotal. +-- ============================================================================ + +create entity MyFirstModule.Week ( Label: String ); +create entity MyFirstModule.WeekRow ( RowTotal: Decimal ); +create association MyFirstModule.WeekRow_Week + from MyFirstModule.WeekRow to MyFirstModule.Week; +/ + +create or replace page MyFirstModule.WeekTimesheet +( Title: 'Week', Layout: Atlas_Core.Atlas_Default, Params: { $Week: MyFirstModule.Week } ) +{ + dataview dvWeek (datasource: $Week) { + dynamictext hLabel (content: 'Week') + -- nested list over the association: its rows are WeekRow, not Week + listview lvRows (datasource: $currentObject/MyFirstModule.WeekRow_Week) { + dynamictext wrTotal (Attribute: RowTotal) + } + } +} +/ + +-- INSERT a data-bound widget as a sibling of wrTotal. It must inherit the +-- association destination (WeekRow) as its context, so RowTotal binds to +-- WeekRow.RowTotal. +alter page MyFirstModule.WeekTimesheet { + insert after wrTotal { + dynamictext probe55 (Attribute: RowTotal, Class: 'vdh-probe') + } +} +/ + +describe page MyFirstModule.WeekTimesheet; diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 46f20694c..6dd36b8f7 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -1349,13 +1349,43 @@ func extractEntityFromDataSource(wDoc bson.D) string { return "" } if entityRef := bsonnav.DGetDoc(ds, "EntityRef"); entityRef != nil { + // DirectEntityRef (database source): the entity is named directly. if entity := bsonnav.DGetString(entityRef, "Entity"); entity != "" { return entity } + // IndirectEntityRef (association source, e.g. a ListView bound + // `from association`): the destination entity lives on the LAST + // EntityRefStep, not at EntityRef.Entity. Without this, descending into + // an association-bound list left the context entity unchanged, so a + // bare attribute inserted via ALTER PAGE resolved against the wrong + // (outer) entity and failed the build with CE1613 (FINDINGS #55). + if entity := lastStepDestinationEntity(entityRef); entity != "" { + return entity + } } return "" } +// lastStepDestinationEntity returns the DestinationEntity of the final +// DomainModels$EntityRefStep in an IndirectEntityRef's Steps array (the entity +// an association path ultimately lands on). The Steps array is +// `[<count>, step, step, ...]` — a leading numeric marker followed by the step +// documents — so non-document elements (the marker) are skipped. Returns "" if +// there are no step documents. +func lastStepDestinationEntity(entityRef bson.D) string { + dest := "" + for _, elem := range bsonnav.DGetArrayElements(bsonnav.DGet(entityRef, "Steps")) { + stepDoc, ok := elem.(bson.D) + if !ok { + continue + } + if d := bsonnav.DGetString(stepDoc, "DestinationEntity"); d != "" { + dest = d + } + } + return dest +} + // --------------------------------------------------------------------------- // Widget scope extraction // --------------------------------------------------------------------------- diff --git a/mdl/backend/pagemutator/mutator_test.go b/mdl/backend/pagemutator/mutator_test.go index 5563a951c..b1941ab14 100644 --- a/mdl/backend/pagemutator/mutator_test.go +++ b/mdl/backend/pagemutator/mutator_test.go @@ -1301,3 +1301,71 @@ func TestSetWidgetProperty_EditableIf_Unsupported(t *testing.T) { t.Fatal("expected error setting EditableIf on a container, got nil") } } + +// makeAssociationListView builds a ListView bound to an association source +// (Forms$AssociationSource → IndirectEntityRef → EntityRefStep) whose +// destination entity is destEntity, containing the given children. This mirrors +// the BSON that serializeAssociationSource emits for `from association`. +func makeAssociationListView(name, assoc, destEntity string, children ...bson.D) bson.D { + childArr := bson.A{int32(2)} + for _, c := range children { + childArr = append(childArr, c) + } + return bson.D{ + {Key: "$Type", Value: "Forms$ListView"}, + {Key: "Name", Value: name}, + {Key: "Widgets", Value: childArr}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$AssociationSource"}, + {Key: "EntityRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$IndirectEntityRef"}, + {Key: "Steps", Value: bson.A{ + int32(2), + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: assoc}, + {Key: "DestinationEntity", Value: destEntity}, + }, + }}, + }}, + }}, + } +} + +// TestEnclosingEntity_AssociationSource is the regression for FINDINGS #55: +// ALTER PAGE INSERT/REPLACE into a list bound `from association` dropped the +// inserted widget's attribute binding because the enclosing entity was read +// from DataSource.EntityRef.Entity, which is empty for an association source +// (the destination lives on the last EntityRefStep). The bare attribute then +// resolved against the wrong (outer) entity — CE1613 at build. +func TestEnclosingEntity_AssociationSource(t *testing.T) { + inner := makeWidget("wrTotal", "Forms$DynamicText") + lv := makeAssociationListView("lvRows", "MyFirstModule.WeekRow_Week", "MyFirstModule.WeekRow", inner) + // Nest the association list inside a DataView bound to the outer entity so + // the walker has a wrong candidate (Week) to fall through to on failure. + dvChildren := bson.A{int32(2), lv} + dv := bson.D{ + {Key: "$Type", Value: "Forms$DataView"}, + {Key: "Name", Value: "dvWeek"}, + {Key: "Widgets", Value: dvChildren}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$DataViewSource"}, + {Key: "EntityRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$DirectEntityRef"}, + {Key: "Entity", Value: "MyFirstModule.Week"}, + }}, + }}, + } + rawData := makeRawPage(dv) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + + // Sibling insert after wrTotal → context is wrTotal's enclosing entity, + // which must be the association destination WeekRow, NOT the outer Week. + if got := m.EnclosingEntity("wrTotal"); got != "MyFirstModule.WeekRow" { + t.Errorf("EnclosingEntity(wrTotal) = %q, want MyFirstModule.WeekRow", got) + } + // INSERT INTO the list → children also take the association destination. + if got := m.EnclosingEntityForChildren("lvRows"); got != "MyFirstModule.WeekRow" { + t.Errorf("EnclosingEntityForChildren(lvRows) = %q, want MyFirstModule.WeekRow", got) + } +} From 25b02acdad5d0e3e4a4298dce9f3258a7c27210a Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 04:13:48 +0000 Subject: [PATCH 16/21] =?UTF-8?q?fix(check,describe):=20clear=20two=20micr?= =?UTF-8?q?oflow-check=20false=20positives=20+=20restore=20workflow-action?= =?UTF-8?q?=20describe=20(FINDINGS=20#51=E2=80=9354)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the TimeRegistration retest, all verified against real mxbuild 11.12.1: #53 — MDL048 rejected `retrieve … where [id = '[%CurrentUser%]']`, the standard signed-in-user idiom (mx check → 0 errors). checkXPathIdConstraint now skips a `'[%…%]'` server-token operand; a real stored-id value is still flagged. #52 — MDL045 rejected division whose divisor is an association-attribute path, e.g. `round($a div $obj/Attr * 100)`. The grammar parses div/*/`/` at one precedence level, so `$a div $obj/Attr` mis-nests as `($a div $obj) / Attr` with `Attr` a bare identifier; MDL045 saw the `/` as division. Mendix has no `/` division operator and re-parses the raw `$obj/Attr` as a path — the serialized output preserves the `/` and mx check passes. exprHasSlashDivision now ignores a `/` whose right operand is a bare IdentifierExpr (member navigation). #54 — `describe microflow` printed `-- Empty action` for `set task outcome` (and open user task / notify workflow) under the default modelsdk engine, so a describe→drop→exec round-trip silently dropped it. The write path and describe formatter already handled these; only the modelsdk read case (actionFromGen) was missing. Added SetTaskOutcome/OpenUserTask/NotifyWorkflow read cases. #51 — `create association` erroring on re-run is correct SQL-shaped semantics (not idempotent); the idempotent form `create or modify association` was undiscoverable. Improved the "already exists" error to name it (and `drop association …`). (#50 daysBetween sign and #52's dateTime-literal restriction are genuine Mendix platform behavior — the latter is already surfaced by MDL046 — so no code change.) Tests: TestValidateMicroflow_XPathIdConstraint (CurrentUser token), TestValidateMicroflow_SlashDivision (div-by-assoc), TestActionFromGen_WorkflowActions. Repros: mdl-examples/bug-tests/{52-53-microflow-check-false-positives,54-describe-set-task-outcome,51-create-or-modify-association}.mdl. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 4 ++ .../51-create-or-modify-association.mdl | 27 ++++++++++ .../52-53-microflow-check-false-positives.mdl | 43 ++++++++++++++++ .../54-describe-set-task-outcome.mdl | 36 +++++++++++++ mdl/backend/modelsdk/microflow_action_test.go | 50 +++++++++++++++++++ .../modelsdk/microflow_read_actions.go | 32 ++++++++++++ mdl/executor/cmd_associations.go | 12 +++-- mdl/executor/validate_microflow.go | 20 +++++++- mdl/executor/validate_microflow_div_test.go | 6 +++ mdl/executor/validate_microflow_hints_test.go | 3 ++ 10 files changed, 228 insertions(+), 5 deletions(-) create mode 100644 mdl-examples/bug-tests/51-create-or-modify-association.mdl create mode 100644 mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl create mode 100644 mdl-examples/bug-tests/54-describe-set-task-outcome.mdl diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index cbb988557..3b3c55ca0 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -27,6 +27,10 @@ to the symptom table below, so the next similar issue costs fewer reads. | `ALTER PAGE INSERT`/`REPLACE` into a list bound `from association` produces a widget whose Attribute binds to the WRONG entity (the outer data view's) or nothing — `mxcli check` ✓ but `mx check` fails **CE1613** "The selected attribute 'Module.OuterEntity.Attr' no longer exists"; DESCRIBE masks it by printing only the short attribute name | The ALTER mutator reads the enclosing entity from `DataSource.EntityRef.Entity`, which is only set for a DIRECT ref (database source). An `AssociationSource` stores its destination on the last `DomainModels$EntityRefStep` of an `IndirectEntityRef`, so the list reported no entity and the context stayed at the outer data view — the bare inserted attribute then resolved against that outer entity | `mdl/backend/pagemutator/mutator.go` (`extractEntityFromDataSource` / `lastStepDestinationEntity`) | Also read the `IndirectEntityRef`'s last `EntityRefStep.DestinationEntity` so a `from association` list reports its child entity to INSERT/REPLACE. Unit guard `TestEnclosingEntity_AssociationSource`; repro `mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl`. FINDINGS #55 | | `mx check` fails to LOAD the project — `StorageLoadException: ... 'Module.Name' is not a valid AttributeIdentifier` after a `create`/`change` with a `Module.Assoc = …` member, yet `mxcli exec` reported success | A one-qualifier member (`Module.Name`) that isn't a known association was written as an *attribute* ref, but a one-qualifier name can't be a valid attribute (attributes are bare or `Module.Entity.Attribute`) → unloadable .mpr. Usually the association's `create` failed earlier (non-idempotent) leaving it absent | `mdl/executor/cmd_microflows_builder_actions.go` (`resolveMemberChange`, the "Not an association in the authored module" branch) | When the domain model is available and the one-dot member isn't in `dm.Associations`/`dm.CrossAssociations`, `fb.addError` with an actionable "create the association first" message instead of writing an Attribute. Same-script associations are visible via `GetDomainModel`, so no false positive. Repro `mdl-examples/bug-tests/264-create-member-unknown-association.mdl`. FINDINGS #51 | | Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | +| `mxcli check` rejects a **valid** microflow: **MDL048** on `retrieve … where [id = '[%CurrentUser%]']` (the standard signed-in-user idiom) — but `mx check` → 0 errors | MDL048 targets constraining `id` against a STORED value (String/Long var or plain literal), which Mendix XPath can't do; it also matched the `'[%CurrentUser%]'` **server token**, which Mendix DOES resolve to a GUID | `mdl/executor/validate_microflow.go` (`checkXPathIdConstraint`) | Skip an operand of the form `'[%…%]'` (a resolved token) before flagging. Case still fires for real stored-id values. Test `TestValidateMicroflow_XPathIdConstraint` (CurrentUser case); repro `mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl`. FINDINGS #53 | +| `mxcli check` rejects a **valid** microflow: **MDL045** ("`/` is division") on `round($a div $obj/Attr * 100)` — division whose divisor is an association-attribute path — but `mx check` → 0 errors | The MDL grammar parses `div`/`*`/`/` at one precedence level, so `$a div $obj/Attr` mis-nests as `($a div $obj) / Attr`; MDL045 saw the `/ Attr` as division. But `Attr` is a bare member name — Mendix has no `/` division operator and re-parses the raw `$obj/Attr` as a path (serialized output preserves the `/`, so the build is clean) | `mdl/executor/validate_microflow.go` (`exprHasSlashDivision`) | Don't flag a `/` BinaryExpr whose RIGHT operand is a bare `IdentifierExpr` (member navigation); real division has a numeric/paren/variable divisor. Test `TestValidateMicroflow_SlashDivision` (div-by-assoc cases); repro `mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl`. FINDINGS #52 | +| `describe microflow` prints `-- Empty action` for a `set task outcome` / `open user task` / `notify workflow` statement (default engine); a describe→drop→exec round-trip silently drops it. Legacy engine (`MXCLI_ENGINE=legacy`) describes it fine | The modelsdk read path (`actionFromGen`) had no case for the workflow microflow actions, so they read back as nil → "Empty action". The write path + DESCRIBE formatter already handled them; only the modelsdk read case was missing | `mdl/backend/modelsdk/microflow_read_actions.go` (`actionFromGen`) | Add cases for `genMf.SetTaskOutcomeAction` / `OpenUserTaskAction` / `NotifyWorkflowAction`, mirroring the legacy parsers. Test `TestActionFromGen_WorkflowActions`; repro `mdl-examples/bug-tests/54-describe-set-task-outcome.mdl`. FINDINGS #54 | +| `create association X …` errors "association already exists" on re-run and aborts the script | Correct SQL-shaped semantics (like `CREATE TABLE`) — `create` is not idempotent. The idempotent form is `create or modify association`, but it was undiscoverable from the bare error | `mdl/executor/cmd_associations.go` (the `NewAlreadyExists("association", …)` sites) | Not a code bug in the write path — improve the error to name `create or modify association …` and `drop association …`. Repro `mdl-examples/bug-tests/51-create-or-modify-association.mdl`. FINDINGS #51 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | | `grant view on page` / `grant execute on microflow\|nanoflow` / `grant access on odata\|published rest service` to a role from **another module** passes `mxcli check`/`exec` but fails the Mendix build with **CE0148 "reselect roles"** — the own-module role works | Document access (page/microflow/nanoflow/service `AllowedModuleRoles`) may only reference the document's **own** module roles; Studio Pro's picker only offers those. The grant path wrote `role.Module + "." + role.Name` verbatim with no same-module check (only `validateModuleRole` = role-exists-in-its-module), so a cross-module reference reached the model. The MOVE path already guarded this (`remapDocumentAccessRoles`) — GRANT didn't | `mdl/executor/cmd_security_defaults.go` (`checkDocumentAccessRolesSameModule`) + the 5 grant handlers in `mdl/executor/cmd_security_write.go` | Pre-check each grant: reject when any `role.Module != docModule` with an actionable message (name the doc's module + suggest the own-module role). Reject (don't silently remap) — a GRANT is explicit, so a wrong role/doc shouldn't be substituted. Wired into page/microflow/nanoflow/OData/published-REST grants. Repro `mdl-examples/bug-tests/ce0148-cross-module-grant.mdl` | | CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` | diff --git a/mdl-examples/bug-tests/51-create-or-modify-association.mdl b/mdl-examples/bug-tests/51-create-or-modify-association.mdl new file mode 100644 index 000000000..757759335 --- /dev/null +++ b/mdl-examples/bug-tests/51-create-or-modify-association.mdl @@ -0,0 +1,27 @@ +-- ============================================================================ +-- FINDINGS #51: `create association` is not idempotent — re-running errors +-- ============================================================================ +-- +-- Symptom: re-running `create association X …` failed with "association already +-- exists" and aborted the script, leaving later statements unexecuted. (The +-- downstream cascade — a member ref to the now-missing association corrupting +-- the project — was already fixed under FINDINGS #51 first half / #90.) +-- +-- Resolution: `create association` erroring on an existing association is +-- correct, SQL-shaped semantics (like `CREATE TABLE`). The idempotent form is +-- `create or modify association`, which updates in place. The "already exists" +-- error now names that form (and `drop association …`) so the fix is +-- discoverable. This file demonstrates the idempotent form is re-runnable. +-- ============================================================================ + +create entity TimeReg.ClientReport ( Name: String ); +create entity TimeReg.Period ( Name: String ); +/ + +-- Idempotent: safe to run repeatedly (unlike plain `create association`). +create or modify association TimeReg.ClientReport_Period + from TimeReg.ClientReport to TimeReg.Period; +/ +create or modify association TimeReg.ClientReport_Period + from TimeReg.ClientReport to TimeReg.Period; +/ diff --git a/mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl b/mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl new file mode 100644 index 000000000..88a8aa2a8 --- /dev/null +++ b/mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl @@ -0,0 +1,43 @@ +-- ============================================================================ +-- FINDINGS #52 & #53: two microflow-check false positives that blocked valid, +-- build-clean Mendix expressions +-- ============================================================================ +-- +-- #53 — MDL048 rejected `[id = '[%CurrentUser%]']`, the standard idiom for the +-- signed-in user's id. MDL048 targets constraining `id` against a STORED value +-- (a String/Long variable or plain literal), which Mendix XPath cannot do; but +-- `[%CurrentUser%]` is a server token Mendix resolves to a GUID, so the build is +-- clean. Fix: checkXPathIdConstraint skips a `'[%…%]'` token operand. +-- +-- #52 — MDL045 rejected division whose divisor is an association-attribute path, +-- e.g. `round($matterHours div $matter/BudgetHours * 100)`. The MDL grammar +-- parses `div`/`*`/`/` at one precedence level, so `$a div $obj/Attr` mis-nests +-- as `($a div $obj) / Attr`, and MDL045 saw the `/ Attr` as a division misuse. +-- But `Attr` is a bare member name — Mendix has no `/` division operator, so it +-- re-parses the raw `$obj/Attr` as a path and the expression builds clean +-- (verified with mxbuild 11.12.1). Fix: exprHasSlashDivision ignores a `/` whose +-- right operand is a bare IdentifierExpr (member navigation). +-- +-- Both microflows below pass `mxcli check` (previously errored) AND `mx check`. +-- ============================================================================ + +create entity MyFirstModule.Matter ( BudgetHours: Decimal ); +/ + +-- #53: current-user id token — must NOT trip MDL048. +create or modify microflow MyFirstModule.BT_CurrentUser () returns String as $out +begin + retrieve $me from System.User where [id = '[%CurrentUser%]'] limit 1; + return 'ok'; +end; +/ + +-- #52: division by an association-attribute path — must NOT trip MDL045. +create or modify microflow MyFirstModule.BT_DivByAssocAttr + ( $matterHours: Decimal, $matter: MyFirstModule.Matter ) returns Decimal as $out +begin + declare $budgetPct Decimal = 0; + set $budgetPct = round($matterHours div $matter/BudgetHours * 100); + return $budgetPct; +end; +/ diff --git a/mdl-examples/bug-tests/54-describe-set-task-outcome.mdl b/mdl-examples/bug-tests/54-describe-set-task-outcome.mdl new file mode 100644 index 000000000..2d2ccd964 --- /dev/null +++ b/mdl-examples/bug-tests/54-describe-set-task-outcome.mdl @@ -0,0 +1,36 @@ +-- ============================================================================ +-- FINDINGS #54: `describe microflow` rendered `set task outcome` as +-- "-- Empty action" under the default (modelsdk) engine +-- ============================================================================ +-- +-- Symptom: a microflow containing `set task outcome $UserTask 'Approve';` +-- executed correctly, but DESCRIBE (default engine) omitted the statement and +-- printed `-- Empty action`. A describe→drop→exec round-trip (e.g. to +-- regenerate documentation) therefore silently LOST the outcome-completion +-- statement — the same class of read gap as finding #42 (describe workflow +-- dropping `with (…)`). The legacy engine described it fine. +-- +-- Root cause: the modelsdk read path (actionFromGen) had no case for the +-- workflow microflow actions, so SetTaskOutcome (and OpenUserTask, +-- NotifyWorkflow) fell through to nil → "-- Empty action". The WRITE path and +-- the DESCRIBE formatter already handled them; only the modelsdk read case was +-- missing. +-- +-- Fix: actionFromGen (mdl/backend/modelsdk/microflow_read_actions.go) now +-- reconstructs SetTaskOutcomeAction / OpenUserTaskAction / NotifyWorkflowAction, +-- mirroring the legacy parsers. +-- +-- Verify: exec against an 11.x project, then +-- describe microflow MyFirstModule.BT_SetOutcome +-- must emit `set task outcome $UserTask 'Approve';`, not `-- Empty action`. +-- ============================================================================ + +create or modify microflow MyFirstModule.BT_SetOutcome + ( $UserTask: System.WorkflowUserTask ) returns nothing +begin + set task outcome $UserTask 'Approve'; + return; +end; +/ + +describe microflow MyFirstModule.BT_SetOutcome; diff --git a/mdl/backend/modelsdk/microflow_action_test.go b/mdl/backend/modelsdk/microflow_action_test.go index 89cb18b89..bb7f9e3da 100644 --- a/mdl/backend/modelsdk/microflow_action_test.go +++ b/mdl/backend/modelsdk/microflow_action_test.go @@ -5,6 +5,8 @@ package modelsdkbackend import ( "testing" + "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/modelsdk/codec" "github.com/mendixlabs/mxcli/modelsdk/element" @@ -250,3 +252,51 @@ func TestMicroflowActionToGen_JavaScriptActionCall(t *testing.T) { t.Error("mapping emits the Java-style Value key instead of ParameterValue") } } + +// TestActionFromGen_WorkflowActions guards FINDINGS #54: the modelsdk read path +// had no case for the workflow microflow actions (SET TASK OUTCOME / OPEN USER +// TASK / NOTIFY WORKFLOW), so DESCRIBE rendered them as "-- Empty action" and a +// describe→drop→exec round-trip silently dropped them. Each must survive a +// write→encode→decode→read round-trip (the real describe path) with its fields +// intact. +func TestActionFromGen_WorkflowActions(t *testing.T) { + // readBack encodes the write-path gen element to BSON and decodes it through + // the codec registry — the concrete *genMf.* type the read path sees — then + // runs actionFromGen, exactly as DESCRIBE does. + readBack := func(t *testing.T, action microflows.MicroflowAction) microflows.MicroflowAction { + t.Helper() + raw, err := (&codec.Encoder{}).Encode(microflowActionToGen(action)) + if err != nil { + t.Fatalf("encode: %v", err) + } + el, err := codec.NewDecoder(codec.DefaultRegistry).Decode(bson.Raw(raw)) + if err != nil { + t.Fatalf("decode: %v", err) + } + return actionFromGen(el) + } + + setOutcome := µflows.SetTaskOutcomeAction{OutcomeValue: "Approve", WorkflowTaskVariable: "UserTask"} + setOutcome.ID = "id-set" + if got, ok := readBack(t, setOutcome).(*microflows.SetTaskOutcomeAction); !ok { + t.Fatalf("SetTaskOutcome read back as %T (Empty-action regression)", readBack(t, setOutcome)) + } else if got.OutcomeValue != "Approve" || got.WorkflowTaskVariable != "UserTask" { + t.Errorf("SetTaskOutcome fields lost: %+v", got) + } + + openTask := µflows.OpenUserTaskAction{UserTaskVariable: "UserTask"} + openTask.ID = "id-open" + if got, ok := readBack(t, openTask).(*microflows.OpenUserTaskAction); !ok { + t.Fatalf("OpenUserTask read back as %T", readBack(t, openTask)) + } else if got.UserTaskVariable != "UserTask" { + t.Errorf("OpenUserTask field lost: %+v", got) + } + + notify := µflows.NotifyWorkflowAction{WorkflowVariable: "Wf"} + notify.ID = "id-notify" + if got, ok := readBack(t, notify).(*microflows.NotifyWorkflowAction); !ok { + t.Fatalf("NotifyWorkflow read back as %T (Empty-action regression)", readBack(t, notify)) + } else if got.WorkflowVariable != "Wf" { + t.Errorf("NotifyWorkflow WorkflowVariable lost: %+v", got) + } +} diff --git a/mdl/backend/modelsdk/microflow_read_actions.go b/mdl/backend/modelsdk/microflow_read_actions.go index 40cb78dbb..7f0800ffe 100644 --- a/mdl/backend/modelsdk/microflow_read_actions.go +++ b/mdl/backend/modelsdk/microflow_read_actions.go @@ -436,6 +436,38 @@ func actionFromGen(el element.Element) microflows.MicroflowAction { } return out + case *genMf.SetTaskOutcomeAction: + // SET TASK OUTCOME $UserTask 'Outcome'. Without this case the workflow + // completion action renders "-- Empty action", so a describe→drop→exec + // round-trip silently loses it (FINDINGS #54). Mirrors legacy + // parseSetTaskOutcomeAction. + out := µflows.SetTaskOutcomeAction{ + ErrorHandlingType: microflows.ErrorHandlingType(a.ErrorHandlingType()), + OutcomeValue: a.OutcomeValue(), + WorkflowTaskVariable: a.WorkflowTaskVariable(), + } + out.ID = model.ID(a.ID()) + return out + + case *genMf.OpenUserTaskAction: + // OPEN USER TASK $UserTask. Mirrors legacy parseOpenUserTaskAction. + out := µflows.OpenUserTaskAction{ + ErrorHandlingType: microflows.ErrorHandlingType(a.ErrorHandlingType()), + UserTaskVariable: a.UserTaskVariable(), + } + out.ID = model.ID(a.ID()) + return out + + case *genMf.NotifyWorkflowAction: + // NOTIFY WORKFLOW $Workflow. Mirrors legacy parseNotifyWorkflowAction. + out := µflows.NotifyWorkflowAction{ + ErrorHandlingType: microflows.ErrorHandlingType(a.ErrorHandlingType()), + OutputVariableName: a.OutputVariableName(), + WorkflowVariable: a.WorkflowVariable(), + } + out.ID = model.ID(a.ID()) + return out + default: return nil } diff --git a/mdl/executor/cmd_associations.go b/mdl/executor/cmd_associations.go index fe21d13c1..eb0e87028 100644 --- a/mdl/executor/cmd_associations.go +++ b/mdl/executor/cmd_associations.go @@ -144,12 +144,14 @@ func execCreateAssociation(ctx *ExecContext, s *ast.CreateAssociationStmt) error if !s.CreateOrModify { for _, ca := range dm.CrossAssociations { if ca.Name == s.Name.Name { - return mdlerrors.NewAlreadyExists("association", s.Name.String()) + return mdlerrors.NewAlreadyExistsMsg("association", s.Name.String(), + fmt.Sprintf("association '%s' already exists — use 'create or modify association ...' to update it in place, or 'drop association %s' first", s.Name.String(), s.Name.String())) } } for _, assoc := range dm.Associations { if assoc.Name == s.Name.Name { - return mdlerrors.NewAlreadyExists("association", s.Name.String()) + return mdlerrors.NewAlreadyExistsMsg("association", s.Name.String(), + fmt.Sprintf("association '%s' already exists — use 'create or modify association ...' to update it in place, or 'drop association %s' first", s.Name.String(), s.Name.String())) } } } @@ -173,12 +175,14 @@ func execCreateAssociation(ctx *ExecContext, s *ast.CreateAssociationStmt) error if !s.CreateOrModify { for _, assoc := range dm.Associations { if assoc.Name == s.Name.Name { - return mdlerrors.NewAlreadyExists("association", s.Name.String()) + return mdlerrors.NewAlreadyExistsMsg("association", s.Name.String(), + fmt.Sprintf("association '%s' already exists — use 'create or modify association ...' to update it in place, or 'drop association %s' first", s.Name.String(), s.Name.String())) } } for _, ca := range dm.CrossAssociations { if ca.Name == s.Name.Name { - return mdlerrors.NewAlreadyExists("association", s.Name.String()) + return mdlerrors.NewAlreadyExistsMsg("association", s.Name.String(), + fmt.Sprintf("association '%s' already exists — use 'create or modify association ...' to update it in place, or 'drop association %s' first", s.Name.String(), s.Name.String())) } } } diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index ca6ac05a6..74d86af92 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -506,7 +506,17 @@ func exprHasSlashDivision(expr ast.Expression) bool { switch e := expr.(type) { case *ast.BinaryExpr: if strings.TrimSpace(e.Operator) == "/" { - return true + // A `/` whose RIGHT operand is a bare member name is association/member + // navigation, not division. The MDL grammar parses `div`/`*`/`/` at one + // precedence level, so `$a div $obj/Attr` mis-nests as `($a div $obj) / Attr` + // with `Attr` a bare IdentifierExpr. Mendix has no `/` division operator, so + // it re-parses the raw `$obj/Attr` as a path and the expression builds clean + // (verified with mxbuild). Only a numeric/parenthesized/variable divisor is a + // real division misuse — those are caught here (right operand is not an + // IdentifierExpr) or by the source `/ $var` scan. (FINDINGS #52) + if _, isMemberName := e.Right.(*ast.IdentifierExpr); !isMemberName { + return true + } } return exprHasSlashDivision(e.Left) || exprHasSlashDivision(e.Right) case *ast.UnaryExpr: @@ -578,6 +588,14 @@ var xpathIdConstraintRe = regexp.MustCompile(`(?:^|[^\w./])(?i:id)\s*(?:=|!=|<|> func (v *microflowValidator) checkXPathIdConstraint(variable, xpath string) { for _, m := range xpathIdConstraintRe.FindAllStringSubmatch(xpath, -1) { operand := m[1] + // `[id = '[%CurrentUser%]']` (and other `'[%…%]'` server tokens) is the + // standard, build-clean Mendix idiom for the signed-in user's id — Mendix's + // XPath engine resolves the token to a GUID, so it IS a valid id operand. + // Only a STORED id value (String/Long variable or a plain literal) is the + // unsupported case this rule targets. (FINDINGS #53) + if strings.HasPrefix(operand, "'[%") && strings.HasSuffix(operand, "%]'") { + continue + } if strings.HasPrefix(operand, "$") { // A $-variable operand: flag only when it is a primitive VALUE (a stored // id — String/Long/Integer). An object variable is not in varKinds, so an diff --git a/mdl/executor/validate_microflow_div_test.go b/mdl/executor/validate_microflow_div_test.go index adda920e5..dacffdd85 100644 --- a/mdl/executor/validate_microflow_div_test.go +++ b/mdl/executor/validate_microflow_div_test.go @@ -85,6 +85,12 @@ func TestValidateMicroflow_SlashDivision(t *testing.T) { {"div is fine", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec div $D2;", false}, {"member path is fine", "$O: M.Order", "set $R = $O/M.Order_Cust/Name;", false}, {"spaced member path is fine", "$O: M.Order", "set $R = $O / M.Order_Cust / Name;", false}, + // `div` divisor is an association-attribute path: `$a div $obj/Attr` mis-nests + // under arithmetic precedence as `($a div $obj) / Attr`, but the `/ Attr` is + // member navigation (bare identifier divisor), not division. Mendix re-parses + // the raw `$obj/Attr` as a path and the expression builds clean (FINDINGS #52). + {"div by association attribute is fine", "$mh: Decimal, $matter: M.Matter", "set $R = round($mh div $matter/BudgetHours * 100);", false}, + {"div by association attribute no round is fine", "$mh: Decimal, $matter: M.Matter", "set $R = $mh div $matter/BudgetHours;", false}, // A `/$` sequence inside a string literal is NOT a division misuse. {"slash-dollar inside string literal is fine", "$x: String", "set $R = 'path/$var here';", false}, } diff --git a/mdl/executor/validate_microflow_hints_test.go b/mdl/executor/validate_microflow_hints_test.go index 6640c0725..6f5959333 100644 --- a/mdl/executor/validate_microflow_hints_test.go +++ b/mdl/executor/validate_microflow_hints_test.go @@ -98,6 +98,9 @@ func TestValidateMicroflow_XPathIdConstraint(t *testing.T) { {"id in boolean clause", "[Active = true and id = $Id]", true}, // Comparing id to an OBJECT variable is the valid "exclude self" pattern. {"id not-equals an object var is fine", "[id != $This]", false}, + // The `[%CurrentUser%]` server token is the standard, build-clean idiom for + // the signed-in user's id — Mendix resolves it to a GUID (FINDINGS #53). + {"id equals CurrentUser token is fine", "[id = '[%CurrentUser%]']", false}, {"attribute containing id is fine", "[Valid = true]", false}, {"paidstatus is fine", "[PaidStatus = $x]", false}, {"plain attribute is fine", "[Name = $n]", false}, From b87e1b9c1aa4bd9b340783bb10333c50a837d3eb Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 04:28:55 +0000 Subject: [PATCH 17/21] fix(vscode-mdl): make `make lint` pass under TypeScript 6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bun install` floats typescript to ^6.0.2, which no longer auto-includes every `@types/*` package on disk — `types` must be listed explicitly. Without it tsc reported 43 errors on a clean checkout (`process`, `console`, `setTimeout` unresolved, plus the implicit-any fallout on node callbacks), so `make lint` failed before any change was made. - tsconfig.json: declare `"types": ["node", "vscode"]` - extension.ts: create the output channel with `{ log: true }` so it is a `LogOutputChannel`, which is what vscode-languageclient 10's `LanguageClientOptions.outputChannel` requires CI only runs `make lint-go`, which is why this went unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA --- vscode-mdl/src/extension.ts | 3 ++- vscode-mdl/tsconfig.json | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/vscode-mdl/src/extension.ts b/vscode-mdl/src/extension.ts index 0c1bae589..79f56824a 100644 --- a/vscode-mdl/src/extension.ts +++ b/vscode-mdl/src/extension.ts @@ -21,7 +21,8 @@ const BUILD_TIME = typeof __BUILD_TIME__ !== 'undefined' ? __BUILD_TIME__ : 'dev const GIT_COMMIT = typeof __GIT_COMMIT__ !== 'undefined' ? __GIT_COMMIT__ : 'unknown'; let client: LanguageClient | undefined; -const outputChannel = vscode.window.createOutputChannel('MDL Language Server'); +// { log: true } yields a LogOutputChannel, which is what LanguageClientOptions.outputChannel requires. +const outputChannel = vscode.window.createOutputChannel('MDL Language Server', { log: true }); const MDL_SCHEME = 'mendix-mdl'; diff --git a/vscode-mdl/tsconfig.json b/vscode-mdl/tsconfig.json index 783a6b8dc..37aa7d2d0 100644 --- a/vscode-mdl/tsconfig.json +++ b/vscode-mdl/tsconfig.json @@ -3,6 +3,7 @@ "module": "commonjs", "target": "ES2020", "lib": ["ES2020"], + "types": ["node", "vscode"], "outDir": "out", "rootDir": "src", "sourceMap": true, From a91e7329dfdc711c3d7f308a39e1ef4b390dac90 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 04:52:22 +0000 Subject: [PATCH 18/21] fix(alter-page): resolve microflow/nanoflow datasource entity for INSERT/REPLACE bindings (FINDINGS #55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retest of #55 showed the association + database datasource cases are fixed, but a microflow/nanoflow datasource still bound nothing: inserting/replacing a data-bound widget in a list bound `datasource: microflow …` produced an unbound attribute (mx check CE0402 "No value specified" / CE1613). Unlike a database (direct EntityRef) or association (IndirectEntityRef) source, a MicroflowSource/NanoflowSource stores no entity in its own BSON — the entity is the flow's RETURN type, which lives in the flow document. So the mutator's BSON walk yielded "" and the bare inserted attribute resolved against nothing. Fix: the page mutator's new EnclosingDataSourceFlow returns the qualified name of the microflow/nanoflow governing the target's context — the nearest ENCLOSING datasource for sibling INSERT/REPLACE, or the widget's OWN datasource for INSERT INTO — via findNearestDataSourceDoc, which returns the nearest datasource *doc* so a nearer non-flow source (database/association) correctly shadows an outer flow. The executor (resolveDataSourceFlowEntity) then resolves that qualified name to the flow's return entity via the existing getMicroflowReturnEntityName / getNanoflowReturnEntityName, and uses it as the widget's entity context when the BSON walk found none. Verified on real mxbuild 11.12.1: INSERT and REPLACE into a microflow-sourced ListView now mx check with 0 errors (CE0402 before). New EnclosingDataSourceFlow interface method + mock/mcp impls (mcp is a no-op — its model resolves entities directly). Unit guard TestEnclosingDataSourceFlow (incl. nearer-source shadowing); repro extended in mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 2 +- .../55-alter-page-insert-assoc-binding.mdl | 34 ++++ mdl/backend/mcp/page_mutator.go | 8 + mdl/backend/mock/mock_page_mutator.go | 8 + mdl/backend/mutation.go | 10 ++ mdl/backend/pagemutator/mutator.go | 149 ++++++++++++++++++ mdl/backend/pagemutator/mutator_test.go | 53 +++++++ mdl/executor/cmd_alter_page.go | 42 ++++- 8 files changed, 304 insertions(+), 2 deletions(-) diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 3b3c55ca0..77148a852 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -24,7 +24,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | | A `create page` reported success but the built page is EMPTY — every widget gone — and `mx check` fails with CE1613 "The selected layout 'dummyModule.dummyName' no longer exists" | The page had no `Layout:` clause, so `buildPageV3` created no `LayoutCall`; the widget tree is built into the LayoutCall's placeholder arguments, so with no LayoutCall the widgets have nowhere to attach and are silently dropped. `dummyModule.dummyName` is *Mendix's* placeholder for a missing layout, not something mxcli writes | `mdl/executor/cmd_pages_builder_v3.go` (`buildPageV3`, the `if page.LayoutCall != nil` block) | Reject a page that has body widgets (or placeholder blocks) but no LayoutCall — distinguish "no Layout: clause" from "layout not found" in the message. A Mendix page always needs a layout; snippets (buildSnippetV3) are layout-less and unaffected. Repro `mdl-examples/bug-tests/266-page-without-layout-drops-widgets.mdl` | -| `ALTER PAGE INSERT`/`REPLACE` into a list bound `from association` produces a widget whose Attribute binds to the WRONG entity (the outer data view's) or nothing — `mxcli check` ✓ but `mx check` fails **CE1613** "The selected attribute 'Module.OuterEntity.Attr' no longer exists"; DESCRIBE masks it by printing only the short attribute name | The ALTER mutator reads the enclosing entity from `DataSource.EntityRef.Entity`, which is only set for a DIRECT ref (database source). An `AssociationSource` stores its destination on the last `DomainModels$EntityRefStep` of an `IndirectEntityRef`, so the list reported no entity and the context stayed at the outer data view — the bare inserted attribute then resolved against that outer entity | `mdl/backend/pagemutator/mutator.go` (`extractEntityFromDataSource` / `lastStepDestinationEntity`) | Also read the `IndirectEntityRef`'s last `EntityRefStep.DestinationEntity` so a `from association` list reports its child entity to INSERT/REPLACE. Unit guard `TestEnclosingEntity_AssociationSource`; repro `mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl`. FINDINGS #55 | +| `ALTER PAGE INSERT`/`REPLACE` into a list bound `from association` **or** `datasource: microflow/nanoflow` produces a widget whose Attribute binds to the WRONG entity (the outer data view's) or nothing — `mxcli check` ✓ but `mx check` fails **CE1613** ("attribute no longer exists") or **CE0402** ("No value specified"); DESCRIBE masks it by printing only the short attribute name | The ALTER mutator read the enclosing entity from `DataSource.EntityRef.Entity`, only set for a DIRECT ref (database). An `AssociationSource` stores its destination on the last `DomainModels$EntityRefStep` of an `IndirectEntityRef`; a `MicroflowSource`/`NanoflowSource` stores NO entity at all (its entity is the flow's RETURN type, in the flow document). Either way the list reported no entity, so the context stayed at the outer data view (or empty) | `mdl/backend/pagemutator/mutator.go` (`extractEntityFromDataSource`+`lastStepDestinationEntity` for association; `EnclosingDataSourceFlow`+`findNearestDataSourceDoc` for flows) + `mdl/executor/cmd_alter_page.go` (`resolveDataSourceFlowEntity`) | Association: read the `IndirectEntityRef`'s last `EntityRefStep.DestinationEntity`. Microflow/nanoflow: the mutator returns the nearest-enclosing (or own, for INTO) flow QN — a nearer non-flow source shadows an outer flow — and the executor resolves its return entity via `getMicroflowReturnEntityName`/`getNanoflowReturnEntityName`. Guards `TestEnclosingEntity_AssociationSource`, `TestEnclosingDataSourceFlow`; repro `mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl`. FINDINGS #55 | | `mx check` fails to LOAD the project — `StorageLoadException: ... 'Module.Name' is not a valid AttributeIdentifier` after a `create`/`change` with a `Module.Assoc = …` member, yet `mxcli exec` reported success | A one-qualifier member (`Module.Name`) that isn't a known association was written as an *attribute* ref, but a one-qualifier name can't be a valid attribute (attributes are bare or `Module.Entity.Attribute`) → unloadable .mpr. Usually the association's `create` failed earlier (non-idempotent) leaving it absent | `mdl/executor/cmd_microflows_builder_actions.go` (`resolveMemberChange`, the "Not an association in the authored module" branch) | When the domain model is available and the one-dot member isn't in `dm.Associations`/`dm.CrossAssociations`, `fb.addError` with an actionable "create the association first" message instead of writing an Attribute. Same-script associations are visible via `GetDomainModel`, so no false positive. Repro `mdl-examples/bug-tests/264-create-member-unknown-association.mdl`. FINDINGS #51 | | Runtime `Failed to load model: ... Class 'Workflows$CallMicroflowTask' could not be found` — the WHOLE app won't boot, yet `mxcli check` ✓ and `mx check` → 0 errors | Mendix 11.9 (WOR-2802) split MicroflowBasedActivity into CallMicroflowActivity + AIAgentTaskActivity, renaming the workflow call-microflow on-disk `$Type` from the pre-11.9 `CallMicroflowTask` to `CallMicroflowActivity`. Writing the old name to an 11.9+ project is fatal at boot only. Evidence: 11.6.3 modeler = only Task; 11.10 modeler = both (Task marked "Removed ... WOR-2802" + a conversion routine); 11.10+ runtime jars = only Activity | `mdl/backend/modelsdk/workflow_write.go` (`applyCallMicroflowStorageName`, `useCallMicroflowActivityName`) + legacy `sdk/mpr/writer_workflow.go` (`renameCallMicroflowTypeBSON`) | Version-gate the emitted `$Type` at 11.9 (same boundary as the `HasOwner`→`HasOwnerAttr` gate): build with the legacy name, rewrite the tree to `CallMicroflowActivity` for `pv.IsAtLeast(11,9)`; register codec TypeDefaults + list-marker under BOTH names; read path already folds both into one semantic type. Repro `mdl-examples/bug-tests/263-workflow-callmicroflow-storage-name.mdl`. FINDINGS #39 | | `mxcli check` rejects a **valid** microflow: **MDL048** on `retrieve … where [id = '[%CurrentUser%]']` (the standard signed-in-user idiom) — but `mx check` → 0 errors | MDL048 targets constraining `id` against a STORED value (String/Long var or plain literal), which Mendix XPath can't do; it also matched the `'[%CurrentUser%]'` **server token**, which Mendix DOES resolve to a GUID | `mdl/executor/validate_microflow.go` (`checkXPathIdConstraint`) | Skip an operand of the form `'[%…%]'` (a resolved token) before flagging. Case still fires for real stored-id values. Test `TestValidateMicroflow_XPathIdConstraint` (CurrentUser case); repro `mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl`. FINDINGS #53 | diff --git a/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl b/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl index e449f30e5..8323cd915 100644 --- a/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl +++ b/mdl-examples/bug-tests/55-alter-page-insert-assoc-binding.mdl @@ -56,3 +56,37 @@ alter page MyFirstModule.WeekTimesheet { / describe page MyFirstModule.WeekTimesheet; +/ + +-- ---------------------------------------------------------------------------- +-- Follow-up (retest): a MICROFLOW datasource is the same class of bug. A +-- microflow/nanoflow source stores no entity in its own BSON — the entity is +-- the flow's RETURN type — so the inserted widget bound nothing (CE0402) until +-- the entity was resolved from the flow. Association + database already worked; +-- this closes the microflow/nanoflow case for INSERT and REPLACE alike. +-- ---------------------------------------------------------------------------- + +create microflow MyFirstModule.DS_Rows () returns list of MyFirstModule.WeekRow as $out +begin + retrieve $out from MyFirstModule.WeekRow; + return $out; +end; +/ + +create or replace page MyFirstModule.MfSourced ( Title: 'Mf', Layout: Atlas_Core.Atlas_Default ) +{ + listview lvMf (datasource: microflow MyFirstModule.DS_Rows) { + dynamictext mfTotal (Attribute: RowTotal) + } +} +/ + +-- probe must bind to WeekRow.RowTotal (the microflow's return entity). +alter page MyFirstModule.MfSourced { + insert after mfTotal { + dynamictext probeMf (Attribute: RowTotal, Class: 'vdh-probe') + } +} +/ + +describe page MyFirstModule.MfSourced; diff --git a/mdl/backend/mcp/page_mutator.go b/mdl/backend/mcp/page_mutator.go index 7186b3814..d468a3f0a 100644 --- a/mdl/backend/mcp/page_mutator.go +++ b/mdl/backend/mcp/page_mutator.go @@ -194,6 +194,14 @@ func (m *mcpPageMutator) EnclosingEntityForChildren(widgetRef string) string { return widgetEntity(w) } +// EnclosingDataSourceFlow is a no-op for the MCP/PED backend: its model resolves +// datasource entities directly via widgetEntity, so there is no flow-return +// indirection to unwind. Returns "","" so the executor keeps the entity that +// EnclosingEntity[ForChildren] already produced. +func (m *mcpPageMutator) EnclosingDataSourceFlow(widgetRef string, forChildren bool) (string, string) { + return "", "" +} + // EnclosingEntity returns the entity context that surrounds a widget — the source // entity of the nearest data-bearing ancestor. func (m *mcpPageMutator) EnclosingEntity(widgetRef string) string { diff --git a/mdl/backend/mock/mock_page_mutator.go b/mdl/backend/mock/mock_page_mutator.go index f94fc2f9e..512098cfd 100644 --- a/mdl/backend/mock/mock_page_mutator.go +++ b/mdl/backend/mock/mock_page_mutator.go @@ -36,6 +36,7 @@ type MockPageMutator struct { SetPluggablePropertyFunc func(widgetRef string, propKey string, op backend.PluggablePropertyOp, ctx backend.PluggablePropertyContext) error EnclosingEntityFunc func(widgetRef string) string EnclosingEntityForChildrenFunc func(widgetRef string) string + EnclosingDataSourceFlowFunc func(widgetRef string, forChildren bool) (string, string) WidgetScopeFunc func() map[string]model.ID ParamScopeFunc func() (map[string]model.ID, map[string]string) SaveFunc func() error @@ -177,6 +178,13 @@ func (m *MockPageMutator) EnclosingEntityForChildren(widgetRef string) string { return "" } +func (m *MockPageMutator) EnclosingDataSourceFlow(widgetRef string, forChildren bool) (string, string) { + if m.EnclosingDataSourceFlowFunc != nil { + return m.EnclosingDataSourceFlowFunc(widgetRef, forChildren) + } + return "", "" +} + func (m *MockPageMutator) WidgetScope() map[string]model.ID { if m.WidgetScopeFunc != nil { return m.WidgetScopeFunc() diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index 91bcffb7f..319149924 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -158,6 +158,16 @@ type PageMutator interface { // surrounding enclosing entity. Used for column inserts/replaces. EnclosingEntityForChildren(widgetRef string) string + // EnclosingDataSourceFlow returns the microflow/nanoflow qualified name of the + // datasource governing widgetRef's context, or "","" when that source is not a + // flow (database/association, which EnclosingEntity[ForChildren] already + // resolve). A microflow/nanoflow datasource's entity is its RETURN type, which + // lives in the flow document, not the datasource BSON — so the caller resolves + // the returned qualified name to an entity via the model. forChildren consults + // the widget's OWN datasource (INSERT INTO / column inserts); otherwise the + // nearest ENCLOSING datasource (sibling INSERT BEFORE/AFTER, REPLACE). + EnclosingDataSourceFlow(widgetRef string, forChildren bool) (microflow, nanoflow string) + // WidgetScope returns a map of widget name → unit ID for all widgets in the tree. WidgetScope() map[string]model.ID diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 6dd36b8f7..0f2c98104 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -772,6 +772,47 @@ func (m *Mutator) EnclosingEntity(widgetRef string) string { return findEnclosingEntityContext(m.rawData, widgetRef) } +// EnclosingDataSourceFlow returns the microflow/nanoflow qualified name of the +// datasource that governs widgetRef's context, or "","" when that source is not +// a flow (database/association — EnclosingEntity/EnclosingEntityForChildren +// already resolve those, and a nearer non-flow source shadows an outer flow). +// A microflow/nanoflow datasource's entity is its RETURN type, which lives in +// the flow document rather than the datasource BSON, so the caller resolves the +// returned qualified name to an entity via the model. When forChildren is true +// the widget's OWN datasource is consulted (INSERT INTO / column inserts); +// otherwise the nearest ENCLOSING datasource (sibling INSERT BEFORE/AFTER, +// REPLACE). Without this a widget inserted into a flow-sourced list bound its +// attribute to nothing (CE0402/CE1613). (FINDINGS #55) +func (m *Mutator) EnclosingDataSourceFlow(widgetRef string, forChildren bool) (microflow, nanoflow string) { + if forChildren { + if result := m.widgetFinder(m.rawData, widgetRef); result != nil { + if ds := bsonnav.DGetDoc(result.widget, "DataSource"); ds != nil { + return flowFromDataSourceDoc(ds) + } + } + } + ds, ok := findNearestDataSourceDoc(m.rawData, widgetRef) + if !ok { + return "", "" + } + return flowFromDataSourceDoc(ds) +} + +// flowFromDataSourceDoc extracts the microflow/nanoflow qualified name from a +// widget's "DataSource" sub-document, or "","" when it is not a flow source. +func flowFromDataSourceDoc(ds bson.D) (microflow, nanoflow string) { + if ds == nil { + return "", "" + } + if s := bsonnav.DGetDoc(ds, "MicroflowSettings"); s != nil { + return bsonnav.DGetString(s, "Microflow"), "" + } + if s := bsonnav.DGetDoc(ds, "NanoflowSettings"); s != nil { + return "", bsonnav.DGetString(s, "Nanoflow") + } + return "", "" +} + // EnclosingEntityForChildren returns the entity context that applies to // children of the named widget. For widgets with their own data source // (DataView, DataGrid, ListView, DataGrid2), this is the data source entity. @@ -1343,6 +1384,114 @@ func findEntityContextInChildren(wDoc bson.D, widgetName string, currentEntity s return "" } +// findNearestDataSourceDoc returns the "DataSource" sub-document of the NEAREST +// container enclosing widgetName that declares one, and whether widgetName was +// found at all. Unlike findEnclosingEntityContext — which resolves to an entity +// NAME and so cannot distinguish "found, but the source has no directly-readable +// entity" (a flow source) from "not found in this branch" — this returns the raw +// source doc, letting the caller resolve flow sources via the model. curDS +// carries the nearest enclosing DataSource seen so far, so a nearer non-flow +// source correctly shadows an outer flow. +func findNearestDataSourceDoc(rawData bson.D, widgetName string) (bson.D, bool) { + if formCall := bsonnav.DGetDoc(rawData, "FormCall"); formCall != nil { + for _, arg := range bsonnav.DGetArrayElements(bsonnav.DGet(formCall, "Arguments")) { + argDoc, ok := arg.(bson.D) + if !ok { + continue + } + if ds, found := findNearestDSInWidgets(argDoc, "Widgets", widgetName, nil); found { + return ds, true + } + } + } + if ds, found := findNearestDSInWidgets(rawData, "Widgets", widgetName, nil); found { + return ds, true + } + if widgetContainer := bsonnav.DGetDoc(rawData, "Widget"); widgetContainer != nil { + if ds, found := findNearestDSInWidgets(widgetContainer, "Widgets", widgetName, nil); found { + return ds, true + } + } + return nil, false +} + +func findNearestDSInWidgets(parentDoc bson.D, key string, widgetName string, curDS bson.D) (bson.D, bool) { + for _, elem := range bsonnav.DGetArrayElements(bsonnav.DGet(parentDoc, key)) { + wDoc, ok := elem.(bson.D) + if !ok { + continue + } + if bsonnav.DGetString(wDoc, "Name") == widgetName { + return curDS, true + } + childDS := curDS + if ds := bsonnav.DGetDoc(wDoc, "DataSource"); ds != nil { + childDS = ds + } + if ds, found := findNearestDSInChildren(wDoc, widgetName, childDS); found { + return ds, true + } + } + return nil, false +} + +func findNearestDSInChildren(wDoc bson.D, widgetName string, curDS bson.D) (bson.D, bool) { + typeName := bsonnav.DGetString(wDoc, "$Type") + if ds, found := findNearestDSInWidgets(wDoc, "Widgets", widgetName, curDS); found { + return ds, true + } + if ds, found := findNearestDSInWidgets(wDoc, "FooterWidgets", widgetName, curDS); found { + return ds, true + } + if strings.Contains(typeName, "LayoutGrid") { + for _, row := range bsonnav.DGetArrayElements(bsonnav.DGet(wDoc, "Rows")) { + rowDoc, ok := row.(bson.D) + if !ok { + continue + } + for _, col := range bsonnav.DGetArrayElements(bsonnav.DGet(rowDoc, "Columns")) { + colDoc, ok := col.(bson.D) + if !ok { + continue + } + if ds, found := findNearestDSInWidgets(colDoc, "Widgets", widgetName, curDS); found { + return ds, true + } + } + } + } + for _, tp := range bsonnav.DGetArrayElements(bsonnav.DGet(wDoc, "TabPages")) { + tpDoc, ok := tp.(bson.D) + if !ok { + continue + } + if ds, found := findNearestDSInWidgets(tpDoc, "Widgets", widgetName, curDS); found { + return ds, true + } + } + if controlBar := bsonnav.DGetDoc(wDoc, "ControlBar"); controlBar != nil { + if ds, found := findNearestDSInWidgets(controlBar, "Items", widgetName, curDS); found { + return ds, true + } + } + if strings.Contains(typeName, "CustomWidget") { + if obj := bsonnav.DGetDoc(wDoc, "Object"); obj != nil { + for _, prop := range bsonnav.DGetArrayElements(bsonnav.DGet(obj, "Properties")) { + propDoc, ok := prop.(bson.D) + if !ok { + continue + } + if valDoc := bsonnav.DGetDoc(propDoc, "Value"); valDoc != nil { + if ds, found := findNearestDSInWidgets(valDoc, "Widgets", widgetName, curDS); found { + return ds, true + } + } + } + } + } + return nil, false +} + func extractEntityFromDataSource(wDoc bson.D) string { ds := bsonnav.DGetDoc(wDoc, "DataSource") if ds == nil { diff --git a/mdl/backend/pagemutator/mutator_test.go b/mdl/backend/pagemutator/mutator_test.go index b1941ab14..45da2b295 100644 --- a/mdl/backend/pagemutator/mutator_test.go +++ b/mdl/backend/pagemutator/mutator_test.go @@ -1369,3 +1369,56 @@ func TestEnclosingEntity_AssociationSource(t *testing.T) { t.Errorf("EnclosingEntityForChildren(lvRows) = %q, want MyFirstModule.WeekRow", got) } } + +// makeMicroflowListView builds a ListView bound to a microflow datasource +// (Forms$MicroflowSource → MicroflowSettings.Microflow), mirroring what the +// writer emits for `datasource: microflow …`. +func makeMicroflowListView(name, microflowQN string, children ...bson.D) bson.D { + childArr := bson.A{int32(2)} + for _, c := range children { + childArr = append(childArr, c) + } + return bson.D{ + {Key: "$Type", Value: "Forms$ListView"}, + {Key: "Name", Value: name}, + {Key: "Widgets", Value: childArr}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$MicroflowSource"}, + {Key: "MicroflowSettings", Value: bson.D{ + {Key: "$Type", Value: "Forms$MicroflowSettings"}, + {Key: "Microflow", Value: microflowQN}, + }}, + }}, + } +} + +// TestEnclosingDataSourceFlow guards the microflow-datasource half of FINDINGS +// #55: a widget inside a list bound to a microflow datasource must report that +// microflow's qualified name (so the executor can resolve its return entity), +// and a nearer NON-flow (association/database) source must shadow an outer flow. +func TestEnclosingDataSourceFlow(t *testing.T) { + // Flat: microflow-sourced list at page top level. + inner := makeWidget("wrTotal", "Forms$DynamicText") + lv := makeMicroflowListView("lvRows", "MyFirstModule.DS_Rows", inner) + m := &Mutator{rawData: makeRawPage(lv), widgetFinder: findBsonWidget} + + // Sibling insert after wrTotal → nearest enclosing datasource is the microflow. + if mf, nf := m.EnclosingDataSourceFlow("wrTotal", false); mf != "MyFirstModule.DS_Rows" || nf != "" { + t.Errorf("EnclosingDataSourceFlow(wrTotal) = (%q,%q), want (MyFirstModule.DS_Rows, )", mf, nf) + } + // INSERT INTO the list → the list's OWN datasource is the microflow. + if mf, _ := m.EnclosingDataSourceFlow("lvRows", true); mf != "MyFirstModule.DS_Rows" { + t.Errorf("EnclosingDataSourceFlow(lvRows, forChildren) = %q, want MyFirstModule.DS_Rows", mf) + } + + // A nearer association source must SHADOW an outer microflow: microflow list + // contains an association-bound list; a widget in the inner list reports NO + // flow (its nearest source is the association). + innerAssoc := makeWidget("leaf", "Forms$DynamicText") + assocLV := makeAssociationListView("lvInner", "MyFirstModule.A_B", "MyFirstModule.B", innerAssoc) + outerMf := makeMicroflowListView("lvOuter", "MyFirstModule.DS_Rows", assocLV) + m2 := &Mutator{rawData: makeRawPage(outerMf), widgetFinder: findBsonWidget} + if mf, nf := m2.EnclosingDataSourceFlow("leaf", false); mf != "" || nf != "" { + t.Errorf("nearer association source should shadow outer microflow: got (%q,%q), want empty", mf, nf) + } +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index cb69c3acc..05c60f1f9 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -194,10 +194,20 @@ func applyInsertWidgetMutator(ctx *ExecContext, mutator backend.PageMutator, op // is a sibling, so use its enclosing container's context; for INSERT INTO the // target IS the container, so the children take the target's own context (e.g. // a dataview's entity). + into := strings.EqualFold(op.Position, "INTO") entityCtx := mutator.EnclosingEntity(op.Target.Widget) - if strings.EqualFold(op.Position, "INTO") { + if into { entityCtx = mutator.EnclosingEntityForChildren(op.Target.Widget) } + // A microflow/nanoflow datasource contributes no entity to the BSON walk (its + // entity is the flow's RETURN type), so resolve it via the model — otherwise a + // widget inserted into a flow-sourced list binds nothing (CE0402/CE1613). (#55) + if entityCtx == "" { + mfQN, nfQN := mutator.EnclosingDataSourceFlow(op.Target.Widget, into) + if e := resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mfQN, nfQN); e != "" { + entityCtx = e + } + } // Build new widgets from AST widgets, err := buildWidgetsFromAST(ctx, op.Widgets, moduleName, moduleID, entityCtx, mutator) @@ -246,6 +256,13 @@ func applyReplaceWidgetMutator(ctx *ExecContext, mutator backend.PageMutator, op // Find entity context from enclosing DataView/DataGrid/ListView for regular widget replace. entityCtx := mutator.EnclosingEntity(op.Target.Widget) + // Resolve a microflow/nanoflow datasource's return entity (see the INSERT path). + if entityCtx == "" { + mfQN, nfQN := mutator.EnclosingDataSourceFlow(op.Target.Widget, false) + if e := resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mfQN, nfQN); e != "" { + entityCtx = e + } + } // Build new widgets from AST, excluding the target widget/column from the // duplicate-name scope so a same-name replacement is allowed. @@ -312,6 +329,29 @@ func buildColumnSpecsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleNa // Widget building from AST (domain logic stays in executor) // ============================================================================ +// resolveDataSourceFlowEntity resolves the entity context contributed by a +// microflow/nanoflow datasource — its RETURN entity — for ALTER PAGE widget +// builds. A flow datasource stores no entity in its own BSON (the entity lives +// in the flow document), so the BSON walk yields "" and a bare inserted +// attribute would bind nothing. Returns "" when neither flow qualified name +// resolves (e.g. a void-returning flow). (FINDINGS #55) +func resolveDataSourceFlowEntity(ctx *ExecContext, moduleName string, moduleID model.ID, mfQN, nfQN string) string { + if mfQN == "" && nfQN == "" { + return "" + } + pb := &pageBuilder{ + ctx: ctx, + backend: ctx.Backend, + moduleID: moduleID, + moduleName: moduleName, + execCache: ctx.Cache, + } + if mfQN != "" { + return pb.getMicroflowReturnEntityName(mfQN) + } + return pb.getNanoflowReturnEntityName(nfQN) +} + // buildWidgetsFromAST converts AST widgets to pages.Widget domain objects. // It uses the mutator for scope resolution (WidgetScope, ParamScope). // excludeFromScope removes named widgets from the duplicate-detection scope, From 0b4feb7765c3cab00242eb5688145650db266722 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 04:51:47 +0000 Subject: [PATCH 19/21] fix(settings): overlay server configurations instead of rebuilding them (#801) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALTER SETTINGS — any section, not just CONFIGURATION — serialized every ServerConfiguration from the semantic model, which carries only the fields mxcli understands. Everything else in the stored document was therefore silently deleted on write: - CustomSettings replaced with an empty array - Tracing replaced with null - OpenAdminPort / OpenHttpPort reset to false (the reader never populates them, so the model always held the zero value) - the Configurations / ConstantValues / CustomSettings version markers downgraded from 3 to the hardcoded 2 - constant overrides rewritten with a flat "Value", the shape Studio Pro and mxbuild ignore — so after one ALTER SETTINGS every override looked empty in Studio Pro, and Integer/Long constants failed the build The configurations are now overlaid onto the raw document they were read from (ADR-0005 guard-don't-drop, which the surrounding settings parts already followed): only fields the read path populates are written, each list keeps its stored marker, and a constant override is updated in the slot it already occupies so a nested SharedOrPrivateValue survives. A new override — which has no stored shape to preserve — is written nested, since that is what the platform reads. A configuration created by CREATE CONFIGURATION takes its shape from a sibling, with the per-configuration collections emptied and a fresh $ID. The overlay lives in mdl/settingsoverlay because both write engines had the same bug in duplicated form; sharing it keeps the codec engine (mdl/backend/modelsdk) and the legacy engine (sdk/mpr) from drifting again. Also refuse the write outright when no raw parts were captured on read: that path would have replaced every settings part with an empty array. Known limitation: a project whose overrides were already flattened by an earlier mxcli run keeps the flat shape, since the overlay preserves what is stored rather than converting it. Those overrides need to be re-entered in Studio Pro once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA --- .claude/skills/fix-issue.md | 1 + ...alter-settings-preserves-configuration.mdl | 52 +++ mdl/backend/modelsdk/settings_write.go | 83 +---- .../settings_write_configuration_test.go | 344 ++++++++++++++++++ mdl/settingsoverlay/settingsoverlay.go | 236 ++++++++++++ mdl/settingsoverlay/settingsoverlay_test.go | 297 +++++++++++++++ sdk/mpr/writer_id_order_test.go | 5 +- sdk/mpr/writer_settings.go | 70 +--- 8 files changed, 960 insertions(+), 128 deletions(-) create mode 100644 mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl create mode 100644 mdl/backend/modelsdk/settings_write_configuration_test.go create mode 100644 mdl/settingsoverlay/settingsoverlay.go create mode 100644 mdl/settingsoverlay/settingsoverlay_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index d351f945e..6e6d7cbdf 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -16,6 +16,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Symptom | Root cause layer | First file to open | Fix pattern | |---------|-----------------|-------------------|-------------| +| Any `ALTER SETTINGS` (any section) reports success but the Default configuration's **Custom settings** are gone, **Tracing** is reset, and every **constant override** shows blank in Studio Pro — Integer/Long constants then fail the build | The configuration was re-serialized from `model.ServerConfiguration`, which carries only the modelled fields, so CustomSettings/Tracing/OpenAdminPort/OpenHttpPort were dropped, the list version markers downgraded 3→2, and overrides were written with a flat `Value` instead of the nested `SharedOrPrivateValue` the platform reads | `mdl/settingsoverlay/settingsoverlay.go` (`Configurations`, `ServerConfiguration`, `ConstantValues`) — called by both `mdl/backend/modelsdk/settings_write.go` and `sdk/mpr/writer_settings.go` | Overlay onto the raw document instead of rebuilding: write only the fields the read path populates, take each list's marker from what is stored, and update a constant override in the slot it already occupies (nested if nested, flat if flat). New override → nested. New configuration → clone a sibling's shape, empty its collections, mint a fresh `$ID`. Refuse the write when `RawParts` is empty. Repro `mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl`. Issue #801 | | MCP op reports `MCP error -32000: Request timed out` but the page/document/entity EXISTS in Studio Pro afterwards | Studio Pro's ~30s server-side per-call limit fires while the op still applies — a client false failure, not a server rejection | `mdl/backend/mcp/timeout.go` (`isTimeoutErr`, `timeoutVerifyDelay`, `pedUpdateVerify`, `pedDocumentExists`) | Verify-on-timeout, never blind-retry a non-idempotent op: idempotent root-replace (`pgWritePage`) retries once; creates confirm via `ped_find_document`; entity adds confirm via a shallow `/entities` read. Unverified → error with save-before-re-run guidance (a blind re-run from a fresh session duplicates elements) | | Retrieve/datasource XPath with a `[%…%]` token (e.g. `[System.owner = '[%CurrentUser%]']` or `[Title = '[%CurrentUser%]']`) fails `mx check` CE0161, but `[Title='abc']` is clean | NOT a token-storage bug — tokens store intact and a type-valid token (`[DueDate < '[%CurrentDateTime%]']`) passes. The failures are semantically-invalid XPath: (1) String/scalar attr compared to a User token = type mismatch; (2) `System.owner`/`changedBy`/… referenced on an entity that doesn't store it (needs `alter entity X add attribute owner: autoowner`) | `mdl/executor/validate.go` (`validateRetrieveConstraints`, `baseSystemMemberRe`) — diagnose with `mxcli bson dump --type microflow` + `mx check`; verify the token alone works | Don't "fix" storage — it's correct. Add a `--references` check: collect retrieve `(entity, constraint)` in `flowRefCollector`, look up the entity via `buildEntityIndex` (`ListDomainModels`), and flag a base-entity `System.<member>` ref (regex excludes `/`-traversed refs) when the entity flag (`HasOwner` etc.) is off, with the `alter entity … add attribute …: auto…` hint. Same-script-created entities aren't in the project index, so the check only fires against existing project entities. **Also fixed**: a bare `[%token%]` *inside* a bracketed constraint (`[DueDate < [%CurrentDateTime%]]`) stored unquoted (the inline path keeps the raw source) → CE0161. `normalizeXPathTokens` (`mdl/visitor/visitor_page_v3.go`) requotes bare tokens; wired into `buildXPathSourceExpression`, the multi-predicate `predicateSources`, `buildXPathString`, and `bracketedXPathFromExpr` (already-quoted tokens untouched). Issue #641 | | `describe` shows `$var = list operation ...;` | Missing parser case | `sdk/mpr/parser_microflow.go` → `parseListOperation()` | Add `case "microflows$XxxType":` returning the correct struct | diff --git a/mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl b/mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl new file mode 100644 index 000000000..3bbaaab93 --- /dev/null +++ b/mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl @@ -0,0 +1,52 @@ +-- Bug #801: ALTER SETTINGS silently corrupted every server configuration +-- +-- Symptom: any ALTER SETTINGS — not just the CONFIGURATION section — rebuilt each +-- Settings$ServerConfiguration from the semantic model, which carries only the +-- fields mxcli understands. Everything else in the stored document was deleted: +-- +-- * CustomSettings → emptied +-- * Tracing → null +-- * OpenAdminPort / +-- OpenHttpPort → false (never read into the model, so always the zero value) +-- * list markers → downgraded from 3 to a hardcoded 2 +-- * constant overrides → rewritten with a flat "Value", the shape Studio Pro and +-- mxbuild ignore, so after one ALTER SETTINGS every override looked empty in +-- Studio Pro and Integer/Long constants failed the build +-- +-- Fix: the configurations are overlaid onto the raw document they were read from +-- (mdl/settingsoverlay, shared by both write engines). Only fields the read path +-- populates are written, each list keeps its stored marker, and a constant override +-- is updated in the slot it already occupies. +-- +-- Manual verification (needs a project, so it is not part of `make check-mdl`'s +-- syntax pass): +-- +-- 1. In Studio Pro, add a custom setting and set a constant override on the +-- Default configuration, then save and close. +-- 2. Run this script: mxcli exec 801-alter-settings-preserves-configuration.mdl -p app.mpr +-- 3. Reopen in Studio Pro: the custom setting and the override are still there, +-- and the override still shows its value. +-- 4. `mx check app.mpr` reports no new errors. +-- +-- Before the fix, step 3 showed an empty Custom settings tab and a blank override. + +create module Issue801; +create module role Issue801.User; + +create constant Issue801.ApiBaseUrl + type string + default 'https://example.invalid'; + +-- Set an override, then touch an unrelated section. Before the fix the second +-- statement was enough on its own to flatten the override written by the first. +alter settings constant 'Issue801.ApiBaseUrl' value 'https://acceptance.example.invalid' + in configuration 'Default'; + +alter settings configuration 'Default' + HttpPortNumber = 8080; + +alter settings model + BcryptCost = 10; + +-- The override survives both writes and still reports its value. +describe settings; diff --git a/mdl/backend/modelsdk/settings_write.go b/mdl/backend/modelsdk/settings_write.go index dbdcae56e..3812c8ac9 100644 --- a/mdl/backend/modelsdk/settings_write.go +++ b/mdl/backend/modelsdk/settings_write.go @@ -8,22 +8,10 @@ import ( "go.mongodb.org/mongo-driver/bson" "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" "github.com/mendixlabs/mxcli/model" ) -// safeInt64 converts an int to int64 with a guard against the float64 safe-integer -// range (settings values are tiny, but keep the conversion bounds-checked). -func safeInt64(v int) int64 { - const maxSafe = 1 << 53 - if v > maxSafe { - return maxSafe - } - if v < -maxSafe { - return -maxSafe - } - return int64(v) -} - // UpdateProjectSettings rewrites the Settings$ProjectSettings unit using the // raw-part overlay strategy (ADR-0005 guard-don't-drop): the Settings array is // rebuilt from ps.RawParts (captured on read), and only the parsed-and-modified @@ -37,6 +25,13 @@ func (b *Backend) UpdateProjectSettings(ps *model.ProjectSettings) error { if b.writer == nil { return fmt.Errorf("UpdateProjectSettings: not connected for writing") } + // Without the raw parts there is nothing to overlay onto, and writing the + // document anyway would replace every settings part with an empty array — + // the whole Project Settings dialog silently reset. Refuse instead. + if len(ps.RawParts) == 0 { + return fmt.Errorf("UpdateProjectSettings: no raw settings parts captured on read; " + + "refusing to write a settings document that would drop every part") + } settings := bson.A{int32(2)} // versioned array prefix for _, rawPart := range ps.RawParts { @@ -50,7 +45,7 @@ func (b *Backend) UpdateProjectSettings(ps *model.ProjectSettings) error { } case "Settings$ConfigurationSettings": if ps.Configuration != nil { - settings = append(settings, overlayConfigurationSettings(ps.Configuration, rawPart)) + settings = append(settings, settingsoverlay.Configurations(ps.Configuration, rawPart)) } else { settings = append(settings, rawPart) } @@ -64,8 +59,8 @@ func (b *Backend) UpdateProjectSettings(ps *model.ProjectSettings) error { case "Settings$WorkflowsProjectSettingsPart": if ps.Workflows != nil { rawPart["UserEntity"] = ps.Workflows.UserEntity - rawPart["DefaultTaskParallelism"] = safeInt64(ps.Workflows.DefaultTaskParallelism) - rawPart["WorkflowEngineParallelism"] = safeInt64(ps.Workflows.WorkflowEngineParallelism) + rawPart["DefaultTaskParallelism"] = settingsoverlay.SafeInt64(ps.Workflows.DefaultTaskParallelism) + rawPart["WorkflowEngineParallelism"] = settingsoverlay.SafeInt64(ps.Workflows.WorkflowEngineParallelism) settings = append(settings, rawPart) } else { settings = append(settings, rawPart) @@ -96,65 +91,13 @@ func overlayModelSettings(ms *model.ModelSettings, raw map[string]any) map[strin raw["HealthCheckMicroflow"] = ms.HealthCheckMicroflow raw["AllowUserMultipleSessions"] = ms.AllowUserMultipleSessions raw["HashAlgorithm"] = ms.HashAlgorithm - raw["BcryptCost"] = safeInt64(ms.BcryptCost) + raw["BcryptCost"] = settingsoverlay.SafeInt64(ms.BcryptCost) raw["JavaVersion"] = ms.JavaVersion raw["RoundingMode"] = ms.RoundingMode raw["ScheduledEventTimeZoneCode"] = ms.ScheduledEventTimeZoneCode raw["FirstDayOfWeek"] = ms.FirstDayOfWeek - raw["DecimalScale"] = safeInt64(ms.DecimalScale) + raw["DecimalScale"] = settingsoverlay.SafeInt64(ms.DecimalScale) raw["EnableDataStorageOptimisticLocking"] = ms.EnableDataStorageOptimisticLocking raw["UseDatabaseForeignKeyConstraints"] = ms.UseDatabaseForeignKeyConstraints return raw } - -func overlayConfigurationSettings(cs *model.ConfigurationSettings, raw map[string]any) map[string]any { - configs := bson.A{int32(2)} // versioned array prefix - for _, cfg := range cs.Configurations { - configs = append(configs, serverConfigurationToBSON(cfg)) - } - raw["Configurations"] = configs - return raw -} - -func serverConfigurationToBSON(cfg *model.ServerConfiguration) bson.M { - cfgDoc := bson.M{ - "$ID": configID(cfg.ID), - "$Type": "Settings$ServerConfiguration", - "Name": cfg.Name, - "DatabaseType": cfg.DatabaseType, - "DatabaseUrl": cfg.DatabaseUrl, - "DatabaseName": cfg.DatabaseName, - "DatabaseUserName": cfg.DatabaseUserName, - "DatabasePassword": cfg.DatabasePassword, - "DatabaseUseIntegratedSecurity": cfg.DatabaseUseIntegratedSecurity, - "HttpPortNumber": safeInt64(cfg.HttpPortNumber), - "ServerPortNumber": safeInt64(cfg.ServerPortNumber), - "ApplicationRootUrl": cfg.ApplicationRootUrl, - "MaxJavaHeapSize": safeInt64(cfg.MaxJavaHeapSize), - "ExtraJvmParameters": cfg.ExtraJvmParameters, - "OpenAdminPort": cfg.OpenAdminPort, - "OpenHttpPort": cfg.OpenHttpPort, - "CustomSettings": bson.A{int32(2)}, - "Tracing": nil, - } - cvArr := bson.A{int32(2)} // versioned array prefix - for _, cv := range cfg.ConstantValues { - cvArr = append(cvArr, bson.M{ - "$ID": configID(cv.ID), - "$Type": "Settings$ConstantValue", - "ConstantId": cv.ConstantId, - "Value": cv.Value, - }) - } - cfgDoc["ConstantValues"] = cvArr - return cfgDoc -} - -// configID returns the binary-UUID $ID for a settings sub-element, generating a -// fresh one when the model carries no ID (a newly-added configuration/constant). -func configID(id model.ID) any { - if id != "" { - return bsonutil.IDToBsonBinary(string(id)) - } - return bsonutil.NewIDBsonBinary() -} diff --git a/mdl/backend/modelsdk/settings_write_configuration_test.go b/mdl/backend/modelsdk/settings_write_configuration_test.go new file mode 100644 index 000000000..50d72c23c --- /dev/null +++ b/mdl/backend/modelsdk/settings_write_configuration_test.go @@ -0,0 +1,344 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" + "github.com/mendixlabs/mxcli/model" + mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" +) + +// The fixture project's Default configuration has no constant overrides, no custom +// settings and a null Tracing, so seeding is needed to observe what a write drops. +// The seeded shapes mirror what Studio Pro stores: +// +// CustomSettings: [3, {Settings$CustomSetting Name/Value}, …] +// Tracing: {Settings$Tracing …} +// ConstantValues: [3, {Settings$ConstantValue ConstantId, SharedOrPrivateValue: +// {Settings$SharedValue Value}}] +// +// The overlay treats CustomSettings and Tracing as opaque, so their exact field +// layout does not matter to the assertions — only that they survive unchanged. +const ( + seededConstantID = "MyFirstModule.SeededConstant" + seededConstValue = "seeded-value" +) + +// seedConfiguration rewrites the fixture's Settings$ProjectSettings unit, adding +// custom settings, a Tracing document and a nested-shape constant override to the +// Default configuration. It returns the configuration document as seeded. +func seedConfiguration(t *testing.T, proj string) map[string]any { + t.Helper() + + r, err := mmpr.OpenWithOptions(proj, mmpr.OpenOptions{ReadOnly: false}) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + refs, err := r.ListUnitsByType("Settings$ProjectSettings") + if err != nil || len(refs) != 1 { + r.Close() + t.Fatalf("ListUnitsByType(Settings$ProjectSettings) = %v, %v", refs, err) + } + unitID := refs[0].ID + raw, err := r.GetRawUnitBytes(unitID) + if err != nil { + r.Close() + t.Fatalf("GetRawUnitBytes: %v", err) + } + var doc bson.M + if err := bson.Unmarshal(raw, &doc); err != nil { + r.Close() + t.Fatalf("unmarshal: %v", err) + } + + var seeded map[string]any + for _, part := range settingsoverlay.ArrayElements(doc["Settings"]) { + if part["$Type"] != "Settings$ConfigurationSettings" { + continue + } + cfgs := settingsoverlay.ArrayElements(part["Configurations"]) + if len(cfgs) == 0 { + r.Close() + t.Fatalf("fixture has no configurations to seed") + } + cfg := cfgs[0] + cfg["CustomSettings"] = bson.A{ + int32(3), + bson.M{ + "$ID": bsonutil.NewIDBsonBinary(), + "$Type": "Settings$CustomSetting", + "Name": "MicroflowConstraintsDisabled", + "Value": "true", + }, + } + cfg["Tracing"] = bson.M{ + "$ID": bsonutil.NewIDBsonBinary(), + "$Type": "Settings$Tracing", + "Level": "Feedback", + } + cfg["ConstantValues"] = bson.A{ + int32(3), + bson.M{ + "$ID": bsonutil.NewIDBsonBinary(), + "$Type": "Settings$ConstantValue", + "ConstantId": seededConstantID, + "SharedOrPrivateValue": bson.M{ + "$ID": bsonutil.NewIDBsonBinary(), + "$Type": "Settings$SharedValue", + "Value": seededConstValue, + }, + }, + } + seeded = cfg + } + if seeded == nil { + r.Close() + t.Fatalf("fixture has no Settings$ConfigurationSettings part") + } + + contents, err := bson.Marshal(bsonutil.OrderStorageValue(doc)) + if err != nil { + r.Close() + t.Fatalf("marshal seeded settings: %v", err) + } + w := mmpr.NewWriterWithReader(r) + if err := w.UpdateRawUnit(unitID, contents); err != nil { + r.Close() + t.Fatalf("UpdateRawUnit: %v", err) + } + if err := r.Close(); err != nil { + t.Fatalf("close fixture: %v", err) + } + return seeded +} + +// readConfiguration returns the raw Default configuration document from disk. +func readConfiguration(t *testing.T, proj string) map[string]any { + t.Helper() + + r, err := mmpr.Open(proj) + if err != nil { + t.Fatalf("open: %v", err) + } + defer r.Close() + refs, err := r.ListUnitsByType("Settings$ProjectSettings") + if err != nil || len(refs) != 1 { + t.Fatalf("ListUnitsByType = %v, %v", refs, err) + } + raw, err := r.GetRawUnitBytes(refs[0].ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + var doc bson.M + if err := bson.Unmarshal(raw, &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + for _, part := range settingsoverlay.ArrayElements(doc["Settings"]) { + if part["$Type"] != "Settings$ConfigurationSettings" { + continue + } + cfgs := settingsoverlay.ArrayElements(part["Configurations"]) + if len(cfgs) == 0 { + t.Fatalf("no configurations on disk") + } + return cfgs[0] + } + t.Fatalf("no Settings$ConfigurationSettings part on disk") + return nil +} + +// TestUpdateProjectSettings_PreservesConfigurationData is the regression test for +// mendixlabs/mxcli#801: any ALTER SETTINGS rebuilt each ServerConfiguration from +// the semantic model, which emptied CustomSettings, nulled Tracing, reset the +// unmodelled Open*Port flags, downgraded the list version markers from 3 to 2, and +// rewrote constant overrides into the flat "Value" shape the platform ignores. +func TestUpdateProjectSettings_PreservesConfigurationData(t *testing.T) { + proj := copyFixture(t) + before := seedConfiguration(t, proj) + wantOpenAdminPort, ok := before["OpenAdminPort"].(bool) + if !ok || !wantOpenAdminPort { + t.Fatalf("fixture precondition: expected OpenAdminPort=true, got %#v", before["OpenAdminPort"]) + } + + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + if ps.Configuration == nil || len(ps.Configuration.Configurations) == 0 { + t.Fatalf("no configuration read back") + } + cfg := ps.Configuration.Configurations[0] + if len(cfg.ConstantValues) != 1 || cfg.ConstantValues[0].Value != seededConstValue { + t.Fatalf("seeded constant override not read: %#v", cfg.ConstantValues) + } + + // The narrowest possible edit: change one modelled scalar, as `ALTER SETTINGS + // CONFIGURATION 'Default' (HttpPortNumber: 8123)` would. + cfg.HttpPortNumber = 8123 + if err := b.UpdateProjectSettings(ps); err != nil { + t.Fatalf("UpdateProjectSettings: %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + after := readConfiguration(t, proj) + + // The edit landed. + if got := after["HttpPortNumber"]; got != int64(8123) && got != int32(8123) { + t.Errorf("HttpPortNumber = %#v, want 8123", got) + } + + // CustomSettings survived, marker included. + customs := settingsoverlay.ArrayElements(after["CustomSettings"]) + if len(customs) != 1 || customs[0]["Name"] != "MicroflowConstraintsDisabled" { + t.Errorf("CustomSettings dropped: %#v", after["CustomSettings"]) + } + if m := settingsoverlay.ArrayMarker(after["CustomSettings"], -1); m != 3 { + t.Errorf("CustomSettings marker = %d, want 3", m) + } + + // Tracing survived as a document rather than being nulled. + tracing, ok := settingsoverlay.AsMap(after["Tracing"]) + if !ok { + t.Errorf("Tracing = %#v, want the seeded document", after["Tracing"]) + } else if tracing["Level"] != "Feedback" { + t.Errorf("Tracing.Level = %#v, want Feedback", tracing["Level"]) + } + + // The unmodelled port flags were not reset to their zero values. + if after["OpenAdminPort"] != true { + t.Errorf("OpenAdminPort = %#v, want true (not read into the model, must pass through)", after["OpenAdminPort"]) + } + + // The constant override kept its nested storage shape and its value. + if m := settingsoverlay.ArrayMarker(after["ConstantValues"], -1); m != 3 { + t.Errorf("ConstantValues marker = %d, want 3", m) + } + cvs := settingsoverlay.ArrayElements(after["ConstantValues"]) + if len(cvs) != 1 { + t.Fatalf("ConstantValues = %#v, want 1 override", after["ConstantValues"]) + } + if cvs[0]["ConstantId"] != seededConstantID { + t.Errorf("ConstantId = %#v, want %s", cvs[0]["ConstantId"], seededConstantID) + } + shared, ok := settingsoverlay.AsMap(cvs[0]["SharedOrPrivateValue"]) + if !ok { + t.Fatalf("SharedOrPrivateValue dropped, override rewritten as %#v", cvs[0]) + } + if shared["Value"] != seededConstValue { + t.Errorf("SharedOrPrivateValue.Value = %#v, want %s", shared["Value"], seededConstValue) + } + if shared["$Type"] != "Settings$SharedValue" { + t.Errorf("SharedOrPrivateValue.$Type = %#v, want Settings$SharedValue", shared["$Type"]) + } +} + +// TestUpdateProjectSettings_ConstantOverrideRoundTrip covers the two write paths for +// a constant override: updating one that already exists on disk (nested shape kept) +// and adding one that does not (nested shape written, since the platform ignores a +// flat "Value"). +func TestUpdateProjectSettings_ConstantOverrideRoundTrip(t *testing.T) { + proj := copyFixture(t) + seedConfiguration(t, proj) + + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + cfg := ps.Configuration.Configurations[0] + cfg.ConstantValues[0].Value = "updated-value" + added := &model.ConstantValue{ConstantId: "MyFirstModule.AddedConstant", Value: "added-value"} + added.TypeName = "Settings$ConstantValue" + cfg.ConstantValues = append(cfg.ConstantValues, added) + if err := b.UpdateProjectSettings(ps); err != nil { + t.Fatalf("UpdateProjectSettings: %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + + after := readConfiguration(t, proj) + cvs := settingsoverlay.ArrayElements(after["ConstantValues"]) + if len(cvs) != 2 { + t.Fatalf("ConstantValues = %#v, want 2 overrides", after["ConstantValues"]) + } + byID := map[string]map[string]any{} + for _, cv := range cvs { + id, _ := cv["ConstantId"].(string) + byID[id] = cv + } + for id, want := range map[string]string{ + seededConstantID: "updated-value", + "MyFirstModule.AddedConstant": "added-value", + } { + cv, ok := byID[id] + if !ok { + t.Errorf("override %s missing from %#v", id, byID) + continue + } + shared, ok := settingsoverlay.AsMap(cv["SharedOrPrivateValue"]) + if !ok { + t.Errorf("override %s has no SharedOrPrivateValue: %#v", id, cv) + continue + } + if shared["Value"] != want { + t.Errorf("override %s value = %#v, want %s", id, shared["Value"], want) + } + if flat, has := cv["Value"]; has && flat != "" { + t.Errorf("override %s has a non-empty flat Value %#v alongside the nested one", id, flat) + } + } + + // The change is visible to the reader too. + b2 := New() + if err := b2.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b2.Disconnect() }) + ps2, err := b2.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings(2): %v", err) + } + got := map[string]string{} + for _, cv := range ps2.Configuration.Configurations[0].ConstantValues { + got[cv.ConstantId] = cv.Value + } + if got[seededConstantID] != "updated-value" || got["MyFirstModule.AddedConstant"] != "added-value" { + t.Errorf("overrides read back as %#v", got) + } +} + +// TestUpdateProjectSettings_RefusesWithoutRawParts guards the ADR-0005 +// guard-don't-drop contract: with no captured raw parts the write would replace +// every settings part with an empty array. +func TestUpdateProjectSettings_RefusesWithoutRawParts(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + ps.RawParts = nil + if err := b.UpdateProjectSettings(ps); err == nil { + t.Fatal("UpdateProjectSettings with no RawParts succeeded; want a refusal") + } +} diff --git a/mdl/settingsoverlay/settingsoverlay.go b/mdl/settingsoverlay/settingsoverlay.go new file mode 100644 index 000000000..c993cdc5f --- /dev/null +++ b/mdl/settingsoverlay/settingsoverlay.go @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package settingsoverlay writes a project's server configurations back onto the +// BSON they were read from, instead of rebuilding them from the semantic model. +// +// model.ServerConfiguration carries only the fields mxcli understands. A +// configuration on disk carries more: CustomSettings, Tracing, the OpenAdminPort / +// OpenHttpPort flags the reader never populates, and whatever a newer Mendix adds. +// Serializing from the model alone therefore silently deletes all of it, and +// rewrites each constant override into the flat "Value" shape that Studio Pro and +// mxbuild ignore (mendixlabs/mxcli#801). Overlaying onto the preserved document is +// the ADR-0005 guard-don't-drop form of the same write. +// +// Both write engines share this package so the two cannot drift apart again: the +// codec engine (mdl/backend/modelsdk) and the legacy engine (sdk/mpr). +package settingsoverlay + +import ( + "strings" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/model" +) + +// DefaultListMarker is the leading version marker Studio Pro writes for the +// settings child lists (Configurations / ConstantValues / CustomSettings), matching +// the codec's default in modelsdk/codec.lookupListMarker. It is only a fallback: an +// existing list keeps whatever marker it already carries, since rebuilding one with +// a hardcoded marker silently downgrades it. +const DefaultListMarker = int32(3) + +// SafeInt64 converts an int to int64 with a guard against the float64 safe-integer +// range (settings values are tiny, but keep the conversion bounds-checked). +func SafeInt64(v int) int64 { + const maxSafe = 1 << 53 + if v > maxSafe { + return maxSafe + } + if v < -maxSafe { + return -maxSafe + } + return int64(v) +} + +// Configurations rebuilds the Configurations list of a raw +// Settings$ConfigurationSettings part, overlaying each modelled configuration onto +// the raw configuration it was read from. Configurations are matched by name, which +// is also how the executor resolves them (ALTER SETTINGS CONFIGURATION '<name>'); +// a modelled configuration with no match on disk is treated as newly created. +// +// raw is mutated and returned. +func Configurations(cs *model.ConfigurationSettings, raw map[string]any) map[string]any { + rawConfigs := ArrayElements(raw["Configurations"]) + byName := make(map[string]map[string]any, len(rawConfigs)) + for _, rc := range rawConfigs { + name, _ := rc["Name"].(string) + byName[strings.ToLower(name)] = rc + } + + configs := bson.A{ArrayMarker(raw["Configurations"], DefaultListMarker)} + for _, cfg := range cs.Configurations { + configs = append(configs, ServerConfiguration(cfg, byName[strings.ToLower(cfg.Name)], rawConfigs)) + } + raw["Configurations"] = configs + return raw +} + +// ServerConfiguration writes the fields the read path populates onto a +// configuration's preserved raw document, leaving every other key untouched. When +// raw is nil the configuration has no counterpart on disk (CREATE CONFIGURATION) +// and a fresh document is derived from siblings; pass nil siblings when there are +// none. +func ServerConfiguration(cfg *model.ServerConfiguration, raw map[string]any, siblings []map[string]any) map[string]any { + if raw == nil { + raw = newServerConfiguration(cfg, siblings) + } + raw["$Type"] = "Settings$ServerConfiguration" + if raw["$ID"] == nil { + raw["$ID"] = elementID(cfg.ID) + } + raw["Name"] = cfg.Name + raw["DatabaseType"] = cfg.DatabaseType + raw["DatabaseUrl"] = cfg.DatabaseUrl + raw["DatabaseName"] = cfg.DatabaseName + raw["DatabaseUserName"] = cfg.DatabaseUserName + raw["DatabasePassword"] = cfg.DatabasePassword + raw["DatabaseUseIntegratedSecurity"] = cfg.DatabaseUseIntegratedSecurity + raw["HttpPortNumber"] = SafeInt64(cfg.HttpPortNumber) + raw["ServerPortNumber"] = SafeInt64(cfg.ServerPortNumber) + raw["ApplicationRootUrl"] = cfg.ApplicationRootUrl + raw["MaxJavaHeapSize"] = SafeInt64(cfg.MaxJavaHeapSize) + raw["ExtraJvmParameters"] = cfg.ExtraJvmParameters + raw["ConstantValues"] = ConstantValues(cfg.ConstantValues, raw["ConstantValues"]) + return raw +} + +// newServerConfiguration builds the raw document for a configuration that has no +// counterpart on disk. A sibling configuration is the shape template when one +// exists — it carries the exact field set and list markers this project's Mendix +// version writes — with the per-configuration collections emptied and the $ID +// dropped so a fresh one is minted. Tracing is inherited from the sibling +// deliberately: its shape is version-specific and a new configuration has no better +// default to offer. Falls back to the field set observed in Studio Pro 10/11 +// projects when the project has no configuration at all. +func newServerConfiguration(cfg *model.ServerConfiguration, siblings []map[string]any) map[string]any { + if len(siblings) > 0 { + tmpl := make(map[string]any, len(siblings[0])) + for k, v := range siblings[0] { + tmpl[k] = v + } + delete(tmpl, "$ID") + tmpl["ConstantValues"] = bson.A{ArrayMarker(siblings[0]["ConstantValues"], DefaultListMarker)} + tmpl["CustomSettings"] = bson.A{ArrayMarker(siblings[0]["CustomSettings"], DefaultListMarker)} + return tmpl + } + return map[string]any{ + "ConstantValues": bson.A{DefaultListMarker}, + "CustomSettings": bson.A{DefaultListMarker}, + "Tracing": nil, + "OpenAdminPort": cfg.OpenAdminPort, + "OpenHttpPort": cfg.OpenHttpPort, + } +} + +// ConstantValues rebuilds a configuration's ConstantValues list, updating each +// override in the slot it is already stored in so its value shape survives. +// Studio Pro and mxbuild only read the nested SharedOrPrivateValue; a flat "Value" +// is a legacy shape mxcli's reader tolerates but the platform ignores, so writing +// every override flat made all constant overrides look empty in Studio Pro. +func ConstantValues(cvs []*model.ConstantValue, raw any) bson.A { + rawValues := ArrayElements(raw) + byConstant := make(map[string]map[string]any, len(rawValues)) + for _, rcv := range rawValues { + id, _ := rcv["ConstantId"].(string) + if id == "" { + // The gen type binds the reference under "Constant"; Studio Pro writes + // "ConstantId" (see modelsdk settings_read.configurationSettingsFromGen). + id, _ = rcv["Constant"].(string) + } + if id != "" { + byConstant[id] = rcv + } + } + + out := bson.A{ArrayMarker(raw, DefaultListMarker)} + for _, cv := range cvs { + out = append(out, constantValue(cv, byConstant[cv.ConstantId])) + } + return out +} + +func constantValue(cv *model.ConstantValue, raw map[string]any) map[string]any { + if raw == nil { + // A new override has no stored shape to preserve, so write the one the + // platform reads: the value nested in a Settings$SharedValue. + return map[string]any{ + "$ID": elementID(cv.ID), + "$Type": "Settings$ConstantValue", + "ConstantId": cv.ConstantId, + "SharedOrPrivateValue": map[string]any{ + "$ID": bsonutil.NewIDBsonBinary(), + "$Type": "Settings$SharedValue", + "Value": cv.Value, + }, + } + } + if shared, ok := AsMap(raw["SharedOrPrivateValue"]); ok { + shared["Value"] = cv.Value + raw["SharedOrPrivateValue"] = shared + // Clear a flat sibling rather than leave the two disagreeing: the reader + // prefers a non-empty flat Value and would report a stale override. + if _, has := raw["Value"]; has { + raw["Value"] = "" + } + return raw + } + // Legacy flat-only shape: keep it flat rather than change the stored shape. + raw["Value"] = cv.Value + return raw +} + +// ArrayMarker returns the leading version marker of a stored versioned array, +// falling back to def when the array is absent or unmarked. +func ArrayMarker(v any, def int32) int32 { + arr, ok := v.(bson.A) + if !ok || len(arr) == 0 { + return def + } + switch m := arr[0].(type) { + case int32: + return m + case int64: + return int32(m) + case int: + return int32(m) + } + return def +} + +// ArrayElements returns the document elements of a stored versioned array; the +// leading marker and any other non-document element are skipped. +func ArrayElements(v any) []map[string]any { + arr, ok := v.(bson.A) + if !ok { + return nil + } + out := make([]map[string]any, 0, len(arr)) + for _, el := range arr { + if m, ok := AsMap(el); ok { + out = append(out, m) + } + } + return out +} + +// AsMap normalises the two shapes bson.Unmarshal produces for a sub-document. +func AsMap(v any) (map[string]any, bool) { + switch m := v.(type) { + case bson.M: + return map[string]any(m), true + case map[string]any: + return m, true + } + return nil, false +} + +// elementID returns the binary-UUID $ID for a settings sub-element, generating a +// fresh one when the model carries no ID (a newly-added configuration/override). +func elementID(id model.ID) any { + if id != "" { + return bsonutil.IDToBsonBinary(string(id)) + } + return bsonutil.NewIDBsonBinary() +} diff --git a/mdl/settingsoverlay/settingsoverlay_test.go b/mdl/settingsoverlay/settingsoverlay_test.go new file mode 100644 index 000000000..248fa4630 --- /dev/null +++ b/mdl/settingsoverlay/settingsoverlay_test.go @@ -0,0 +1,297 @@ +// SPDX-License-Identifier: Apache-2.0 + +package settingsoverlay + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +func TestArrayMarker(t *testing.T) { + tests := []struct { + name string + in any + want int32 + }{ + {"int32 marker", bson.A{int32(3), bson.M{}}, 3}, + {"int64 marker", bson.A{int64(2)}, 2}, + {"int marker", bson.A{5}, 5}, + {"empty array falls back", bson.A{}, 7}, + {"not an array falls back", "nope", 7}, + {"nil falls back", nil, 7}, + {"unmarked array falls back", bson.A{bson.M{"$Type": "x"}}, 7}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := ArrayMarker(tc.in, 7); got != tc.want { + t.Errorf("ArrayMarker(%#v) = %d, want %d", tc.in, got, tc.want) + } + }) + } +} + +func TestArrayElements_SkipsMarkerAndScalars(t *testing.T) { + in := bson.A{int32(3), bson.M{"Name": "a"}, "junk", map[string]any{"Name": "b"}} + got := ArrayElements(in) + if len(got) != 2 { + t.Fatalf("ArrayElements returned %d elements, want 2: %#v", len(got), got) + } + if got[0]["Name"] != "a" || got[1]["Name"] != "b" { + t.Errorf("ArrayElements = %#v", got) + } +} + +// TestServerConfiguration_PreservesUnknownKeys pins the core of #801: keys the +// semantic model does not carry must pass through the overlay untouched. +func TestServerConfiguration_PreservesUnknownKeys(t *testing.T) { + raw := map[string]any{ + "$ID": "existing-id-sentinel", + "$Type": "Settings$ServerConfiguration", + "Name": "Default", + "OpenAdminPort": true, + "OpenHttpPort": true, + "CustomSettings": bson.A{int32(3), bson.M{"Name": "Foo", "Value": "1"}}, + "Tracing": bson.M{"$Type": "Settings$Tracing", "Level": "Feedback"}, + "SomeFutureKey": "keep me", + "ConstantValues": bson.A{int32(3)}, + } + cfg := &model.ServerConfiguration{Name: "Default", HttpPortNumber: 8123} + + got := ServerConfiguration(cfg, raw, nil) + + if got["OpenAdminPort"] != true || got["OpenHttpPort"] != true { + t.Errorf("Open*Port reset: OpenAdminPort=%#v OpenHttpPort=%#v", got["OpenAdminPort"], got["OpenHttpPort"]) + } + if len(ArrayElements(got["CustomSettings"])) != 1 { + t.Errorf("CustomSettings dropped: %#v", got["CustomSettings"]) + } + if _, ok := AsMap(got["Tracing"]); !ok { + t.Errorf("Tracing nulled: %#v", got["Tracing"]) + } + if got["SomeFutureKey"] != "keep me" { + t.Errorf("unknown key dropped: %#v", got["SomeFutureKey"]) + } + if got["$ID"] != "existing-id-sentinel" { + t.Errorf("$ID rewritten: %#v", got["$ID"]) + } + if got["HttpPortNumber"] != int64(8123) { + t.Errorf("HttpPortNumber = %#v, want 8123", got["HttpPortNumber"]) + } + if m := ArrayMarker(got["ConstantValues"], -1); m != 3 { + t.Errorf("ConstantValues marker = %d, want the stored 3", m) + } +} + +func TestConstantValues_UpdatesNestedShapeInPlace(t *testing.T) { + raw := bson.A{ + int32(3), + bson.M{ + "$ID": "cv-id", + "$Type": "Settings$ConstantValue", + "ConstantId": "Mod.C1", + "Value": "stale-flat", + "SharedOrPrivateValue": bson.M{ + "$Type": "Settings$SharedValue", + "Value": "old", + }, + }, + } + got := ConstantValues([]*model.ConstantValue{{ConstantId: "Mod.C1", Value: "new"}}, raw) + + cvs := ArrayElements(got) + if len(cvs) != 1 { + t.Fatalf("got %d overrides, want 1: %#v", len(cvs), got) + } + shared, ok := AsMap(cvs[0]["SharedOrPrivateValue"]) + if !ok { + t.Fatalf("SharedOrPrivateValue dropped: %#v", cvs[0]) + } + if shared["Value"] != "new" { + t.Errorf("nested Value = %#v, want new", shared["Value"]) + } + // The stale flat sibling is cleared so the reader cannot report it instead. + if cvs[0]["Value"] != "" { + t.Errorf("flat Value = %#v, want cleared", cvs[0]["Value"]) + } + if cvs[0]["$ID"] != "cv-id" { + t.Errorf("$ID rewritten: %#v", cvs[0]["$ID"]) + } +} + +func TestConstantValues_KeepsLegacyFlatShape(t *testing.T) { + raw := bson.A{ + int32(3), + bson.M{"$ID": "cv-id", "$Type": "Settings$ConstantValue", "ConstantId": "Mod.C1", "Value": "old"}, + } + cvs := ArrayElements(ConstantValues([]*model.ConstantValue{{ConstantId: "Mod.C1", Value: "new"}}, raw)) + if len(cvs) != 1 { + t.Fatalf("got %d overrides, want 1", len(cvs)) + } + if cvs[0]["Value"] != "new" { + t.Errorf("flat Value = %#v, want new", cvs[0]["Value"]) + } + if _, ok := cvs[0]["SharedOrPrivateValue"]; ok { + t.Errorf("a nested value was added to a flat-only override: %#v", cvs[0]) + } +} + +// TestConstantValues_NewOverrideIsNested: a brand-new override has no stored shape, +// so it must be written in the shape Studio Pro and mxbuild actually read. +func TestConstantValues_NewOverrideIsNested(t *testing.T) { + cvs := ArrayElements(ConstantValues([]*model.ConstantValue{{ConstantId: "Mod.New", Value: "v"}}, bson.A{int32(3)})) + if len(cvs) != 1 { + t.Fatalf("got %d overrides, want 1", len(cvs)) + } + if cvs[0]["ConstantId"] != "Mod.New" { + t.Errorf("ConstantId = %#v", cvs[0]["ConstantId"]) + } + shared, ok := AsMap(cvs[0]["SharedOrPrivateValue"]) + if !ok { + t.Fatalf("new override is not nested: %#v", cvs[0]) + } + if shared["Value"] != "v" || shared["$Type"] != "Settings$SharedValue" { + t.Errorf("SharedOrPrivateValue = %#v", shared) + } + if _, ok := cvs[0]["Value"]; ok { + t.Errorf("new override also wrote a flat Value: %#v", cvs[0]) + } +} + +// TestConstantValues_MatchesByConstantKey covers the "Constant" spelling the gen type +// binds, alongside the "ConstantId" Studio Pro writes. +func TestConstantValues_MatchesByConstantKey(t *testing.T) { + raw := bson.A{ + int32(3), + bson.M{"$Type": "Settings$ConstantValue", "Constant": "Mod.C1", "Value": "old"}, + } + cvs := ArrayElements(ConstantValues([]*model.ConstantValue{{ConstantId: "Mod.C1", Value: "new"}}, raw)) + if len(cvs) != 1 || cvs[0]["Value"] != "new" { + t.Errorf("override matched by Constant key not updated in place: %#v", cvs) + } +} + +func TestConfigurations_MatchesByNameAndDrops(t *testing.T) { + part := map[string]any{ + "$Type": "Settings$ConfigurationSettings", + "Configurations": bson.A{ + int32(3), + bson.M{"$ID": "id-default", "Name": "Default", "SomeFutureKey": "a"}, + bson.M{"$ID": "id-prod", "Name": "Production", "SomeFutureKey": "b"}, + }, + } + cs := &model.ConfigurationSettings{ + // Production dropped; Default matched case-insensitively. + Configurations: []*model.ServerConfiguration{{Name: "default", HttpPortNumber: 9000}}, + } + + got := Configurations(cs, part) + cfgs := ArrayElements(got["Configurations"]) + if len(cfgs) != 1 { + t.Fatalf("got %d configurations, want 1 (Production dropped): %#v", len(cfgs), got["Configurations"]) + } + if cfgs[0]["$ID"] != "id-default" { + t.Errorf("matched the wrong raw configuration: %#v", cfgs[0]) + } + if cfgs[0]["SomeFutureKey"] != "a" { + t.Errorf("unknown key dropped on the matched configuration: %#v", cfgs[0]) + } + if m := ArrayMarker(got["Configurations"], -1); m != 3 { + t.Errorf("Configurations marker = %d, want the stored 3", m) + } +} + +// TestConfigurations_NewConfigurationUsesSiblingShape: CREATE CONFIGURATION has no +// raw document, so the shape is taken from a sibling — minus its identity and its +// per-configuration collections. +func TestConfigurations_NewConfigurationUsesSiblingShape(t *testing.T) { + part := map[string]any{ + "Configurations": bson.A{ + int32(3), + bson.M{ + "$ID": "id-default", + "Name": "Default", + "OpenAdminPort": true, + "CustomSettings": bson.A{int32(3), bson.M{"Name": "Foo"}}, + "ConstantValues": bson.A{int32(3), bson.M{"ConstantId": "Mod.C1", "Value": "x"}}, + "Tracing": bson.M{"$Type": "Settings$Tracing"}, + }, + }, + } + cs := &model.ConfigurationSettings{ + Configurations: []*model.ServerConfiguration{ + {Name: "Default"}, + {Name: "Acceptance", DatabaseType: "PostgreSQL"}, + }, + } + + cfgs := ArrayElements(Configurations(cs, part)["Configurations"]) + if len(cfgs) != 2 { + t.Fatalf("got %d configurations, want 2", len(cfgs)) + } + var added map[string]any + for _, c := range cfgs { + if c["Name"] == "Acceptance" { + added = c + } + } + if added == nil { + t.Fatalf("new configuration not written: %#v", cfgs) + } + if added["$ID"] == nil || added["$ID"] == "id-default" { + t.Errorf("new configuration must get a fresh $ID, got %#v", added["$ID"]) + } + if added["OpenAdminPort"] != true { + t.Errorf("sibling shape not inherited: OpenAdminPort=%#v", added["OpenAdminPort"]) + } + if got := ArrayElements(added["CustomSettings"]); len(got) != 0 { + t.Errorf("new configuration inherited the sibling's custom settings: %#v", got) + } + if got := ArrayElements(added["ConstantValues"]); len(got) != 0 { + t.Errorf("new configuration inherited the sibling's constant overrides: %#v", got) + } + if added["DatabaseType"] != "PostgreSQL" { + t.Errorf("DatabaseType = %#v, want PostgreSQL", added["DatabaseType"]) + } + // The sibling itself is untouched by having been used as a template. + for _, c := range cfgs { + if c["Name"] != "Default" { + continue + } + if len(ArrayElements(c["CustomSettings"])) != 1 { + t.Errorf("template sibling lost its custom settings: %#v", c["CustomSettings"]) + } + } +} + +// TestServerConfiguration_NoSiblings covers a project with no configuration at all: +// the fallback field set must still carry the keys Studio Pro expects. +func TestServerConfiguration_NoSiblings(t *testing.T) { + got := ServerConfiguration(&model.ServerConfiguration{Name: "Default", HttpPortNumber: 8080}, nil, nil) + for _, key := range []string{"$ID", "$Type", "Name", "CustomSettings", "ConstantValues", "OpenAdminPort", "OpenHttpPort"} { + if _, ok := got[key]; !ok { + t.Errorf("fallback configuration is missing %q: %#v", key, got) + } + } + if _, ok := got["Tracing"]; !ok { + t.Errorf("fallback configuration is missing Tracing: %#v", got) + } + if m := ArrayMarker(got["CustomSettings"], -1); m != DefaultListMarker { + t.Errorf("CustomSettings marker = %d, want %d", m, DefaultListMarker) + } +} + +func TestSafeInt64_Bounds(t *testing.T) { + const maxSafe = int64(1) << 53 + if got := SafeInt64(8080); got != 8080 { + t.Errorf("SafeInt64(8080) = %d", got) + } + if got := SafeInt64(int(maxSafe) + 10); got != maxSafe { + t.Errorf("SafeInt64 above range = %d, want clamp to %d", got, maxSafe) + } + if got := SafeInt64(-int(maxSafe) - 10); got != -maxSafe { + t.Errorf("SafeInt64 below range = %d, want clamp to %d", got, -maxSafe) + } +} diff --git a/sdk/mpr/writer_id_order_test.go b/sdk/mpr/writer_id_order_test.go index 8cae0d6d3..32cf1552b 100644 --- a/sdk/mpr/writer_id_order_test.go +++ b/sdk/mpr/writer_id_order_test.go @@ -6,6 +6,8 @@ import ( "fmt" "testing" + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" @@ -225,6 +227,7 @@ func TestStorageObjects_IDIsFirstProperty(t *testing.T) { {ConstantId: "Mod.C1", Value: "v"}, }, } - marshalAndValidate(t, "ServerConfiguration", serializeServerConfiguration(cfg)) + marshalAndValidate(t, "ServerConfiguration", + bsonutil.OrderStorageValue(settingsoverlay.ServerConfiguration(cfg, nil, nil))) }) } diff --git a/sdk/mpr/writer_settings.go b/sdk/mpr/writer_settings.go index a597df7c6..7c5e442f1 100644 --- a/sdk/mpr/writer_settings.go +++ b/sdk/mpr/writer_settings.go @@ -5,6 +5,7 @@ package mpr import ( "fmt" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" "github.com/mendixlabs/mxcli/model" "go.mongodb.org/mongo-driver/bson" @@ -30,6 +31,14 @@ func (w *Writer) UpdateProjectSettings(ps *model.ProjectSettings) error { // It uses the RawParts for round-trip fidelity, updating only the parts // that have been parsed and modified. func (w *Writer) serializeProjectSettings(ps *model.ProjectSettings) ([]byte, error) { + // Without the raw parts there is nothing to overlay onto, and writing the + // document anyway would replace every settings part with an empty array — the + // whole Project Settings dialog silently reset. Refuse instead. + if len(ps.RawParts) == 0 { + return nil, fmt.Errorf("no raw settings parts captured on read; " + + "refusing to write a settings document that would drop every part") + } + doc := bson.D{ {Key: "$ID", Value: idToBsonBinary(string(ps.ID))}, {Key: "$Type", Value: "Settings$ProjectSettings"}, @@ -95,64 +104,11 @@ func serializeModelSettings(ms *model.ModelSettings, raw map[string]any) map[str return raw } -// serializeConfigurationSettings updates the raw BSON map with modified configuration settings. +// serializeConfigurationSettings overlays the modified configuration settings onto +// the raw BSON part. The overlay is shared with the codec engine so the two write +// paths cannot drift (see mdl/settingsoverlay and mendixlabs/mxcli#801). func serializeConfigurationSettings(cs *model.ConfigurationSettings, raw map[string]any) map[string]any { - configs := bson.A{int32(2)} // versioned array prefix - for _, cfg := range cs.Configurations { - configs = append(configs, serializeServerConfiguration(cfg)) - } - raw["Configurations"] = configs - return raw -} - -func serializeServerConfiguration(cfg *model.ServerConfiguration) bson.D { - id := string(cfg.ID) - if id == "" { - id = generateUUID() - } - - // Serialize ConstantValues - cvArr := bson.A{int32(2)} // versioned array prefix - for _, cv := range cfg.ConstantValues { - cvArr = append(cvArr, serializeConstantValue(cv)) - } - - cfgDoc := bson.D{ - {Key: "$ID", Value: idToBsonBinary(id)}, - {Key: "$Type", Value: "Settings$ServerConfiguration"}, - {Key: "Name", Value: cfg.Name}, - {Key: "DatabaseType", Value: cfg.DatabaseType}, - {Key: "DatabaseUrl", Value: cfg.DatabaseUrl}, - {Key: "DatabaseName", Value: cfg.DatabaseName}, - {Key: "DatabaseUserName", Value: cfg.DatabaseUserName}, - {Key: "DatabasePassword", Value: cfg.DatabasePassword}, - {Key: "DatabaseUseIntegratedSecurity", Value: cfg.DatabaseUseIntegratedSecurity}, - {Key: "HttpPortNumber", Value: safeInt64(cfg.HttpPortNumber)}, - {Key: "ServerPortNumber", Value: safeInt64(cfg.ServerPortNumber)}, - {Key: "ApplicationRootUrl", Value: cfg.ApplicationRootUrl}, - {Key: "MaxJavaHeapSize", Value: safeInt64(cfg.MaxJavaHeapSize)}, - {Key: "ExtraJvmParameters", Value: cfg.ExtraJvmParameters}, - {Key: "OpenAdminPort", Value: cfg.OpenAdminPort}, - {Key: "OpenHttpPort", Value: cfg.OpenHttpPort}, - {Key: "CustomSettings", Value: bson.A{int32(2)}}, - {Key: "Tracing", Value: nil}, - {Key: "ConstantValues", Value: cvArr}, - } - - return cfgDoc -} - -func serializeConstantValue(cv *model.ConstantValue) bson.D { - id := string(cv.ID) - if id == "" { - id = generateUUID() - } - return bson.D{ - {Key: "$ID", Value: idToBsonBinary(id)}, - {Key: "$Type", Value: "Settings$ConstantValue"}, - {Key: "ConstantId", Value: cv.ConstantId}, - {Key: "Value", Value: cv.Value}, - } + return settingsoverlay.Configurations(cs, raw) } // serializeLanguageSettings updates the raw BSON map with modified language settings. From 4deca963b44427d5e3796d35e3854f6ed27a4a8d Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 05:03:14 +0000 Subject: [PATCH 20/21] fix(settings): reject invalid typed values instead of ignoring them (#805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `alter settings configuration 'Default' HttpPortNumber = 'not-a-number'` printed "Updated configuration 'Default'" and changed nothing. Every Integer-typed setting parsed its value with the error discarded — `if v, err := strconv.Atoi(valStr); err == nil` — so an unparseable value skipped the assignment while the handler still reported success. DESCRIBE SETTINGS then showed the original value. The boolean settings had the same hole in a different form: `AllowUserMultipleSessions = valStr == "true"` mapped every other spelling, including a typo or a plausible 'yes', to false and reported success. Both now parse through helpers that return a validation error naming the setting and the offending value, so nothing is written. Covers all seven sites: BcryptCost, AllowUserMultipleSessions, DefaultTaskParallelism, WorkflowEngineParallelism, and HttpPortNumber / ServerPortNumber on both ALTER SETTINGS CONFIGURATION and CREATE CONFIGURATION. The same values are now reported at check time too (MDL-SET01 integers, MDL-SET02 booleans), wired into `mxcli check` and the LSP so a typo surfaces before the project is opened for writing. TestTypedSettingsKeys_MatchExecutor guards the check-time table against drifting from the executor's assignment switch. Out of scope: range validation. A port of 0 or 999999, or a negative BcryptCost, still parses as an integer and is accepted, as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA --- .claude/skills/fix-issue.md | 1 + .claude/skills/mendix/project-settings.md | 7 + cmd/mxcli/cmd_check.go | 7 + cmd/mxcli/lsp_diagnostics.go | 6 + docs-site/src/tools/builtin-rules.md | 4 +- .../805-alter-settings-typed-values.fail.mdl | 30 ++ mdl/executor/cmd_settings.go | 73 +++- mdl/executor/cmd_settings_validation_test.go | 410 ++++++++++++++++++ mdl/executor/validate_settings.go | 105 +++++ 9 files changed, 627 insertions(+), 16 deletions(-) create mode 100644 mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl create mode 100644 mdl/executor/cmd_settings_validation_test.go create mode 100644 mdl/executor/validate_settings.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 6e6d7cbdf..08fac5937 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -17,6 +17,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Symptom | Root cause layer | First file to open | Fix pattern | |---------|-----------------|-------------------|-------------| | Any `ALTER SETTINGS` (any section) reports success but the Default configuration's **Custom settings** are gone, **Tracing** is reset, and every **constant override** shows blank in Studio Pro — Integer/Long constants then fail the build | The configuration was re-serialized from `model.ServerConfiguration`, which carries only the modelled fields, so CustomSettings/Tracing/OpenAdminPort/OpenHttpPort were dropped, the list version markers downgraded 3→2, and overrides were written with a flat `Value` instead of the nested `SharedOrPrivateValue` the platform reads | `mdl/settingsoverlay/settingsoverlay.go` (`Configurations`, `ServerConfiguration`, `ConstantValues`) — called by both `mdl/backend/modelsdk/settings_write.go` and `sdk/mpr/writer_settings.go` | Overlay onto the raw document instead of rebuilding: write only the fields the read path populates, take each list's marker from what is stored, and update a constant override in the slot it already occupies (nested if nested, flat if flat). New override → nested. New configuration → clone a sibling's shape, empty its collections, mint a fresh `$ID`. Refuse the write when `RawParts` is empty. Repro `mdl-examples/bug-tests/801-alter-settings-preserves-configuration.mdl`. Issue #801 | +| `ALTER SETTINGS` / `CREATE CONFIGURATION` prints "Updated …" but `DESCRIBE SETTINGS` shows the old value — an Integer property was given a non-numeric value, or a Boolean anything other than `true` | `strconv.Atoi`'s error was discarded (`if v, err := …; err == nil`) so the assignment was skipped while the caller still printed success; the boolean form compared against `"true"`, silently mapping every other spelling to false | `mdl/executor/cmd_settings.go` (`settingsInt`, `settingsBool`) + `mdl/executor/validate_settings.go` (`typedSettingsKeys`, MDL-SET01/MDL-SET02) | Parse through a helper that returns a validation error naming the setting and the offending value, and register the property in `typedSettingsKeys` so `mxcli check` and the LSP flag it before the project is opened for writing. `TestTypedSettingsKeys_MatchExecutor` guards the table against drifting from the executor's switch. Repro `mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl`. Issue #805 | | MCP op reports `MCP error -32000: Request timed out` but the page/document/entity EXISTS in Studio Pro afterwards | Studio Pro's ~30s server-side per-call limit fires while the op still applies — a client false failure, not a server rejection | `mdl/backend/mcp/timeout.go` (`isTimeoutErr`, `timeoutVerifyDelay`, `pedUpdateVerify`, `pedDocumentExists`) | Verify-on-timeout, never blind-retry a non-idempotent op: idempotent root-replace (`pgWritePage`) retries once; creates confirm via `ped_find_document`; entity adds confirm via a shallow `/entities` read. Unverified → error with save-before-re-run guidance (a blind re-run from a fresh session duplicates elements) | | Retrieve/datasource XPath with a `[%…%]` token (e.g. `[System.owner = '[%CurrentUser%]']` or `[Title = '[%CurrentUser%]']`) fails `mx check` CE0161, but `[Title='abc']` is clean | NOT a token-storage bug — tokens store intact and a type-valid token (`[DueDate < '[%CurrentDateTime%]']`) passes. The failures are semantically-invalid XPath: (1) String/scalar attr compared to a User token = type mismatch; (2) `System.owner`/`changedBy`/… referenced on an entity that doesn't store it (needs `alter entity X add attribute owner: autoowner`) | `mdl/executor/validate.go` (`validateRetrieveConstraints`, `baseSystemMemberRe`) — diagnose with `mxcli bson dump --type microflow` + `mx check`; verify the token alone works | Don't "fix" storage — it's correct. Add a `--references` check: collect retrieve `(entity, constraint)` in `flowRefCollector`, look up the entity via `buildEntityIndex` (`ListDomainModels`), and flag a base-entity `System.<member>` ref (regex excludes `/`-traversed refs) when the entity flag (`HasOwner` etc.) is off, with the `alter entity … add attribute …: auto…` hint. Same-script-created entities aren't in the project index, so the check only fires against existing project entities. **Also fixed**: a bare `[%token%]` *inside* a bracketed constraint (`[DueDate < [%CurrentDateTime%]]`) stored unquoted (the inline path keeps the raw source) → CE0161. `normalizeXPathTokens` (`mdl/visitor/visitor_page_v3.go`) requotes bare tokens; wired into `buildXPathSourceExpression`, the multi-predicate `predicateSources`, `buildXPathString`, and `bracketedXPathFromExpr` (already-quoted tokens untouched). Issue #641 | | `describe` shows `$var = list operation ...;` | Missing parser case | `sdk/mpr/parser_microflow.go` → `parseListOperation()` | Add `case "microflows$XxxType":` returning the correct struct | diff --git a/.claude/skills/mendix/project-settings.md b/.claude/skills/mendix/project-settings.md index f76371e3c..c3461ce43 100644 --- a/.claude/skills/mendix/project-settings.md +++ b/.claude/skills/mendix/project-settings.md @@ -54,6 +54,12 @@ alter settings configuration 'Default' DatabaseUrl = 'newhost:5432'; ``` +`HttpPortNumber`, `ServerPortNumber`, `BcryptCost`, `DefaultTaskParallelism` and +`WorkflowEngineParallelism` are Integer-typed, and `AllowUserMultipleSessions` is +Boolean. An unparseable value is rejected by `mxcli check` (MDL-SET01 / MDL-SET02) +and by the write itself — it is no longer silently ignored. Quoted numbers are +fine: `HttpPortNumber = '8080'` and `HttpPortNumber = 8080` are equivalent. + ### Constant Overrides ```sql @@ -129,3 +135,4 @@ alter settings configuration 'Default' - [ ] There is always exactly one ProjectSettings document; it cannot be created or deleted - [ ] Model setting key names are case-sensitive (e.g., `JavaVersion`, not `javaversion`) - [ ] Configuration names are case-insensitive (e.g., `'default'` matches `'default'`) +- [ ] Integer / Boolean settings must parse — `mxcli check` reports MDL-SET01 / MDL-SET02 before the write diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 81d02221a..2776c8b78 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -126,6 +126,13 @@ Examples: if wfStmt, ok := stmt.(*ast.CreateWorkflowStmt); ok { violations = append(violations, executor.ValidateWorkflow(wfStmt)...) } + // Check typed ALTER SETTINGS / CREATE CONFIGURATION property values + if setStmt, ok := stmt.(*ast.AlterSettingsStmt); ok { + violations = append(violations, executor.ValidateSettings(setStmt)...) + } + if cfgStmt, ok := stmt.(*ast.CreateConfigurationStmt); ok { + violations = append(violations, executor.ValidateCreateConfiguration(cfgStmt)...) + } // Check view entity OQL if viewStmt, ok := stmt.(*ast.CreateViewEntityStmt); ok { if viewStmt.Query.RawQuery != "" { diff --git a/cmd/mxcli/lsp_diagnostics.go b/cmd/mxcli/lsp_diagnostics.go index afe9ab4bb..c67d00ef1 100644 --- a/cmd/mxcli/lsp_diagnostics.go +++ b/cmd/mxcli/lsp_diagnostics.go @@ -279,6 +279,12 @@ func (s *mdlServer) runSemanticValidation(text string) []protocol.Diagnostic { if mfStmt, ok := stmt.(*ast.CreateMicroflowStmt); ok { violations = append(violations, executor.ValidateMicroflow(mfStmt)...) } + if setStmt, ok := stmt.(*ast.AlterSettingsStmt); ok { + violations = append(violations, executor.ValidateSettings(setStmt)...) + } + if cfgStmt, ok := stmt.(*ast.CreateConfigurationStmt); ok { + violations = append(violations, executor.ValidateCreateConfiguration(cfgStmt)...) + } if viewStmt, ok := stmt.(*ast.CreateViewEntityStmt); ok { if viewStmt.Query.RawQuery != "" { violations = append(violations, executor.ValidateOQLSyntax(viewStmt.Query.RawQuery)...) diff --git a/docs-site/src/tools/builtin-rules.md b/docs-site/src/tools/builtin-rules.md index 57138b006..d94d32bcc 100644 --- a/docs-site/src/tools/builtin-rules.md +++ b/docs-site/src/tools/builtin-rules.md @@ -55,11 +55,13 @@ mxcli lint -p app.mpr --exclude System --exclude Administration ## Check-time Rules (`mxcli check`) -These rules run with `mxcli check` (and the LSP, for real-time diagnostics) rather than `mxcli lint`. They focus on pluggable-widget authoring. +These rules run with `mxcli check` (and the LSP, for real-time diagnostics) rather than `mxcli lint`. They cover pluggable-widget authoring and typed project-settings values. | Rule | Where it fires | Description | |------|----------------|-------------| | **MDL-WIDGET01** | `mxcli check` + LSP | Unknown property key on a pluggable widget. The property is not in the widget's `.def.json`. Catches typos like `optionsSourcType` (missing `e`) before MxBuild does. Suggests the nearest known key. | | **MDL-WIDGET02** | `mxcli check --post-migration` | Legacy native widget found on a project that has a pluggable replacement available. Reports each occurrence with the qualified document name, widget instance name, and the recommended pluggable widget. | +| **MDL-SET01** | `mxcli check` + LSP | Non-integer value for an Integer-typed project setting (`HttpPortNumber`, `ServerPortNumber`, `BcryptCost`, `DefaultTaskParallelism`, `WorkflowEngineParallelism`). These used to be skipped silently while the statement still reported success. | +| **MDL-SET02** | `mxcli check` + LSP | Value other than `true` / `false` for a Boolean-typed project setting (`AllowUserMultipleSessions`). Anything else was silently stored as `false`. | Run `mxcli check --help` for usage. See [Error Messages → MDL-WIDGET01 / MDL-WIDGET02](../appendixes/error-messages.md#mdl-widget01-unknown-pluggable-widget-property) for cause-and-solution detail. diff --git a/mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl b/mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl new file mode 100644 index 000000000..f06afdb0c --- /dev/null +++ b/mdl-examples/bug-tests/805-alter-settings-typed-values.fail.mdl @@ -0,0 +1,30 @@ +-- Bug #805: ALTER SETTINGS silently ignored invalid values for typed properties +-- +-- NEGATIVE TEST (.fail.mdl) — this script is EXPECTED to fail `mxcli check`. +-- `make check-mdl` inverts the exit code for .fail.mdl files: an unexpected +-- pass would be reported as a regression of MDL-SET01 / MDL-SET02. +-- +-- Symptom: `HttpPortNumber = 'not-a-number'` printed "Updated configuration +-- 'Default'" and changed nothing. strconv.Atoi's error was discarded +-- (`if v, err := strconv.Atoi(valStr); err == nil`), so the assignment was +-- skipped while the handler still reported success. DESCRIBE SETTINGS showed +-- the original value. The boolean settings had the same hole: comparing the +-- value against "true" turned every other spelling into false. +-- +-- Fix: the executor returns a validation error naming the setting and the +-- offending value, and `mxcli check` reports the same before the project is +-- opened for writing (MDL-SET01 integers, MDL-SET02 booleans). +-- +-- Expected: `mxcli check` exits non-zero with MDL-SET01 on both port numbers +-- and on BcryptCost, and MDL-SET02 on AllowUserMultipleSessions. + +alter settings configuration 'Default' + HttpPortNumber = 'not-a-number', + ServerPortNumber = '80O0'; + +alter settings model + BcryptCost = 'high', + AllowUserMultipleSessions = 'yes'; + +alter settings workflows + DefaultTaskParallelism = 'many'; diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index b890780b0..8b5b64026 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -197,15 +197,21 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { case "HashAlgorithm": ps.Model.HashAlgorithm = valStr case "BcryptCost": - if v, err := strconv.Atoi(valStr); err == nil { - ps.Model.BcryptCost = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + ps.Model.BcryptCost = v case "JavaVersion": ps.Model.JavaVersion = valStr case "RoundingMode": ps.Model.RoundingMode = valStr case "AllowUserMultipleSessions": - ps.Model.AllowUserMultipleSessions = valStr == "true" + v, err := settingsBool(key, valStr) + if err != nil { + return err + } + ps.Model.AllowUserMultipleSessions = v case "ScheduledEventTimeZoneCode": ps.Model.ScheduledEventTimeZoneCode = valStr default: @@ -240,13 +246,17 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { case "UserEntity": ps.Workflows.UserEntity = valStr case "DefaultTaskParallelism": - if v, err := strconv.Atoi(valStr); err == nil { - ps.Workflows.DefaultTaskParallelism = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + ps.Workflows.DefaultTaskParallelism = v case "WorkflowEngineParallelism": - if v, err := strconv.Atoi(valStr); err == nil { - ps.Workflows.WorkflowEngineParallelism = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + ps.Workflows.WorkflowEngineParallelism = v default: return mdlerrors.NewUnsupported("unknown workflow setting: " + key) } @@ -325,13 +335,17 @@ func alterSettingsConfiguration(ctx *ExecContext, ps *model.ProjectSettings, stm case "DatabasePassword": cfg.DatabasePassword = valStr case "HttpPortNumber": - if v, err := strconv.Atoi(valStr); err == nil { - cfg.HttpPortNumber = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + cfg.HttpPortNumber = v case "ServerPortNumber": - if v, err := strconv.Atoi(valStr); err == nil { - cfg.ServerPortNumber = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + cfg.ServerPortNumber = v case "ApplicationRootUrl": cfg.ApplicationRootUrl = valStr default: @@ -462,13 +476,17 @@ func createConfiguration(ctx *ExecContext, stmt *ast.CreateConfigurationStmt) er case "DatabasePassword": newCfg.DatabasePassword = valStr case "HttpPortNumber": - if v, err := strconv.Atoi(valStr); err == nil { - newCfg.HttpPortNumber = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + newCfg.HttpPortNumber = v case "ServerPortNumber": - if v, err := strconv.Atoi(valStr); err == nil { - newCfg.ServerPortNumber = v + v, err := settingsInt(key, valStr) + if err != nil { + return err } + newCfg.ServerPortNumber = v case "ApplicationRootUrl": newCfg.ApplicationRootUrl = valStr default: @@ -518,6 +536,31 @@ func dropConfiguration(ctx *ExecContext, stmt *ast.DropConfigurationStmt) error return mdlerrors.NewNotFound("configuration", stmt.Name) } +// settingsInt parses an Integer-typed settings value. The error used to be +// discarded (`if v, err := strconv.Atoi(...); err == nil`), so a non-numeric value +// skipped the assignment while the handler still printed its success line — the +// field silently kept its old value (#805). +func settingsInt(key, valStr string) (int, error) { + v, err := strconv.Atoi(strings.TrimSpace(valStr)) + if err != nil { + return 0, mdlerrors.NewValidationf("%s must be an integer, got %q", key, valStr) + } + return v, nil +} + +// settingsBool parses a Boolean-typed settings value. Comparing against "true" +// turned every other spelling — a typo, or a plausible value like 'yes' — into +// false while still reporting success, the same silent no-op as settingsInt. +func settingsBool(key, valStr string) (bool, error) { + switch strings.ToLower(strings.TrimSpace(valStr)) { + case "true": + return true, nil + case "false": + return false, nil + } + return false, mdlerrors.NewValidationf("%s must be true or false, got %q", key, valStr) +} + // settingsValueToString converts an AST settings value to string. func settingsValueToString(val any) string { switch v := val.(type) { diff --git a/mdl/executor/cmd_settings_validation_test.go b/mdl/executor/cmd_settings_validation_test.go new file mode 100644 index 000000000..2570b7c73 --- /dev/null +++ b/mdl/executor/cmd_settings_validation_test.go @@ -0,0 +1,410 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/model" +) + +// settingsBackend returns a MockBackend serving a single 'Default' configuration and +// recording whether a write was attempted, so a rejected statement can be shown to +// be a no-op rather than a silent partial write. +func settingsBackend(wrote *bool) *mock.MockBackend { + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { + ps := &model.ProjectSettings{ + Model: &model.ModelSettings{ + HashAlgorithm: "BCrypt", + BcryptCost: 10, + AllowUserMultipleSessions: true, + }, + Workflows: &model.WorkflowsSettings{ + DefaultTaskParallelism: 5, + WorkflowEngineParallelism: 5, + }, + Configuration: &model.ConfigurationSettings{ + Configurations: []*model.ServerConfiguration{{ + Name: "Default", + HttpPortNumber: 8080, + ServerPortNumber: 8090, + }}, + }, + } + ps.RawParts = []map[string]any{{"$Type": "Settings$ConfigurationSettings"}} + return ps, nil + }, + UpdateProjectSettingsFunc: func(*model.ProjectSettings) error { + *wrote = true + return nil + }, + } +} + +// TestAlterSettings_RejectsNonIntegerValues is the regression test for +// mendixlabs/mxcli#805: strconv.Atoi's error was discarded, so an invalid value for +// an Integer-typed setting skipped the assignment while the handler still reported +// success — a silent no-op. +func TestAlterSettings_RejectsNonIntegerValues(t *testing.T) { + tests := []struct { + name string + stmt *ast.AlterSettingsStmt + key string + }{ + { + name: "configuration HttpPortNumber", + stmt: &ast.AlterSettingsStmt{ + Section: "configuration", + ConfigName: "Default", + Properties: map[string]any{"HttpPortNumber": "not-a-number"}, + }, + key: "HttpPortNumber", + }, + { + name: "configuration ServerPortNumber", + stmt: &ast.AlterSettingsStmt{ + Section: "configuration", + ConfigName: "Default", + Properties: map[string]any{"ServerPortNumber": "8O9O"}, + }, + key: "ServerPortNumber", + }, + { + name: "model BcryptCost", + stmt: &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"BcryptCost": "high"}, + }, + key: "BcryptCost", + }, + { + name: "workflows DefaultTaskParallelism", + stmt: &ast.AlterSettingsStmt{ + Section: "workflows", + Properties: map[string]any{"DefaultTaskParallelism": "many"}, + }, + key: "DefaultTaskParallelism", + }, + { + name: "workflows WorkflowEngineParallelism", + stmt: &ast.AlterSettingsStmt{ + Section: "workflows", + Properties: map[string]any{"WorkflowEngineParallelism": ""}, + }, + key: "WorkflowEngineParallelism", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + wrote := false + ctx, buf := newMockCtx(t, withBackend(settingsBackend(&wrote))) + + err := alterSettings(ctx, tc.stmt) + if err == nil { + t.Fatalf("alterSettings accepted an invalid %s; output was %q", tc.key, buf.String()) + } + if !strings.Contains(err.Error(), tc.key) { + t.Errorf("error does not name the setting: %v", err) + } + if !strings.Contains(err.Error(), "must be an integer") { + t.Errorf("error does not say what was expected: %v", err) + } + if wrote { + t.Errorf("a rejected statement still wrote the settings document") + } + if out := buf.String(); strings.Contains(out, "Updated") { + t.Errorf("a rejected statement still reported success: %q", out) + } + }) + } +} + +// TestAlterSettings_RejectsNonBooleanValues covers the same silent no-op in its +// boolean form: `valStr == "true"` turned anything else into false. +func TestAlterSettings_RejectsNonBooleanValues(t *testing.T) { + for _, val := range []string{"yes", "ture", "1", ""} { + t.Run("AllowUserMultipleSessions="+val, func(t *testing.T) { + wrote := false + ctx, buf := newMockCtx(t, withBackend(settingsBackend(&wrote))) + + err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"AllowUserMultipleSessions": val}, + }) + if err == nil { + t.Fatalf("alterSettings accepted AllowUserMultipleSessions=%q; output was %q", val, buf.String()) + } + if !strings.Contains(err.Error(), "must be true or false") { + t.Errorf("unexpected error: %v", err) + } + if wrote { + t.Errorf("a rejected statement still wrote the settings document") + } + }) + } +} + +// TestAlterSettings_AcceptsValidTypedValues guards against over-rejecting: the valid +// spellings, including a numeric literal from the parser and surrounding whitespace, +// must still be applied and written. +func TestAlterSettings_AcceptsValidTypedValues(t *testing.T) { + tests := []struct { + name string + stmt *ast.AlterSettingsStmt + verify func(*testing.T, *model.ProjectSettings) + }{ + { + name: "quoted integer", + stmt: &ast.AlterSettingsStmt{ + Section: "configuration", + ConfigName: "Default", + Properties: map[string]any{"HttpPortNumber": "8123"}, + }, + verify: func(t *testing.T, ps *model.ProjectSettings) { + if got := ps.Configuration.Configurations[0].HttpPortNumber; got != 8123 { + t.Errorf("HttpPortNumber = %d, want 8123", got) + } + }, + }, + { + name: "numeric literal", + stmt: &ast.AlterSettingsStmt{ + Section: "configuration", + ConfigName: "Default", + Properties: map[string]any{"ServerPortNumber": int64(9090)}, + }, + verify: func(t *testing.T, ps *model.ProjectSettings) { + if got := ps.Configuration.Configurations[0].ServerPortNumber; got != 9090 { + t.Errorf("ServerPortNumber = %d, want 9090", got) + } + }, + }, + { + name: "padded integer", + stmt: &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"BcryptCost": " 12 "}, + }, + verify: func(t *testing.T, ps *model.ProjectSettings) { + if got := ps.Model.BcryptCost; got != 12 { + t.Errorf("BcryptCost = %d, want 12", got) + } + }, + }, + { + name: "boolean false", + stmt: &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"AllowUserMultipleSessions": "false"}, + }, + verify: func(t *testing.T, ps *model.ProjectSettings) { + if ps.Model.AllowUserMultipleSessions { + t.Error("AllowUserMultipleSessions = true, want false") + } + }, + }, + { + name: "boolean literal", + stmt: &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"AllowUserMultipleSessions": true}, + }, + verify: func(t *testing.T, ps *model.ProjectSettings) { + if !ps.Model.AllowUserMultipleSessions { + t.Error("AllowUserMultipleSessions = false, want true") + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + wrote := false + mb := settingsBackend(&wrote) + var written *model.ProjectSettings + mb.UpdateProjectSettingsFunc = func(ps *model.ProjectSettings) error { + wrote = true + written = ps + return nil + } + ctx, _ := newMockCtx(t, withBackend(mb)) + + if err := alterSettings(ctx, tc.stmt); err != nil { + t.Fatalf("alterSettings rejected a valid value: %v", err) + } + if !wrote { + t.Fatal("a valid statement did not write the settings document") + } + tc.verify(t, written) + }) + } +} + +// TestCreateConfiguration_RejectsNonIntegerValues covers the same discarded +// conversion on the CREATE CONFIGURATION path. +func TestCreateConfiguration_RejectsNonIntegerValues(t *testing.T) { + for _, key := range []string{"HttpPortNumber", "ServerPortNumber"} { + t.Run(key, func(t *testing.T) { + wrote := false + ctx, _ := newMockCtx(t, withBackend(settingsBackend(&wrote))) + + err := createConfiguration(ctx, &ast.CreateConfigurationStmt{ + Name: "Acceptance", + Properties: map[string]any{key: "eighty-eighty"}, + }) + if err == nil { + t.Fatalf("createConfiguration accepted an invalid %s", key) + } + if !strings.Contains(err.Error(), key) || !strings.Contains(err.Error(), "must be an integer") { + t.Errorf("unexpected error: %v", err) + } + if wrote { + t.Errorf("a rejected CREATE CONFIGURATION still wrote the settings document") + } + }) + } +} + +// TestValidateSettings_ReportsTypedValueErrors covers the check-time half of the +// fix: `mxcli check` must report the same invalid values the executor rejects, so a +// typo surfaces before the project is opened for writing. +func TestValidateSettings_ReportsTypedValueErrors(t *testing.T) { + tests := []struct { + name string + stmt *ast.AlterSettingsStmt + ruleID string + }{ + { + name: "integer", + stmt: &ast.AlterSettingsStmt{ + Section: "configuration", + ConfigName: "Default", + Properties: map[string]any{"HttpPortNumber": "eighty-eighty"}, + }, + ruleID: "MDL-SET01", + }, + { + name: "boolean", + stmt: &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"AllowUserMultipleSessions": "yes"}, + }, + ruleID: "MDL-SET02", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := ValidateSettings(tc.stmt) + if len(got) != 1 { + t.Fatalf("ValidateSettings returned %d violations, want 1: %+v", len(got), got) + } + if got[0].RuleID != tc.ruleID { + t.Errorf("RuleID = %q, want %q", got[0].RuleID, tc.ruleID) + } + if got[0].Severity != linter.SeverityError { + t.Errorf("Severity = %v, want error", got[0].Severity) + } + if got[0].Suggestion == "" { + t.Error("violation has no suggestion") + } + }) + } +} + +func TestValidateSettings_AcceptsValidAndUnknownSections(t *testing.T) { + valid := ValidateSettings(&ast.AlterSettingsStmt{ + Section: "configuration", + Properties: map[string]any{"HttpPortNumber": "8080", "DatabaseName": "anything goes"}, + }) + if len(valid) != 0 { + t.Errorf("valid properties reported as violations: %+v", valid) + } + // A section with no typed properties (or an unknown one) is not this rule's business. + if got := ValidateSettings(&ast.AlterSettingsStmt{ + Section: "language", + Properties: map[string]any{"DefaultLanguageCode": "en_US"}, + }); len(got) != 0 { + t.Errorf("language section reported violations: %+v", got) + } +} + +func TestValidateCreateConfiguration_ReportsTypedValueErrors(t *testing.T) { + got := ValidateCreateConfiguration(&ast.CreateConfigurationStmt{ + Name: "Acceptance", + Properties: map[string]any{"ServerPortNumber": "nope"}, + }) + if len(got) != 1 || got[0].RuleID != "MDL-SET01" { + t.Fatalf("ValidateCreateConfiguration = %+v, want one MDL-SET01", got) + } + if !strings.Contains(got[0].Message, "Acceptance") { + t.Errorf("message does not name the configuration: %q", got[0].Message) + } +} + +// TestTypedSettingsKeys_MatchExecutor is the drift guard for the hand-maintained +// typedSettingsKeys table: every entry must correspond to a property the executor +// actually parses, otherwise `mxcli check` and `mxcli exec` would disagree about +// what is valid. +func TestTypedSettingsKeys_MatchExecutor(t *testing.T) { + // A value that parses as neither an integer nor a boolean. + const bad = "definitely-not-typed" + + for section, keys := range typedSettingsKeys { + for key, kind := range keys { + t.Run(section+"/"+key, func(t *testing.T) { + // The validator must flag it. + if got := ValidateSettings(&ast.AlterSettingsStmt{ + Section: section, + ConfigName: "Default", + Properties: map[string]any{key: bad}, + }); len(got) != 1 { + t.Errorf("ValidateSettings did not flag %s.%s: %+v", section, key, got) + } + + // And the executor must refuse to write it. + wrote := false + ctx, _ := newMockCtx(t, withBackend(settingsBackend(&wrote))) + err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: section, + ConfigName: "Default", + Properties: map[string]any{key: bad}, + }) + if err == nil { + t.Errorf("executor accepted an invalid %s.%s — table and executor disagree", section, key) + } + if wrote { + t.Errorf("executor wrote settings despite an invalid %s.%s", section, key) + } + + // The valid form for this kind must round-trip through both. + good := "7" + if kind == settingsKindBool { + good = "true" + } + if got := ValidateSettings(&ast.AlterSettingsStmt{ + Section: section, + ConfigName: "Default", + Properties: map[string]any{key: good}, + }); len(got) != 0 { + t.Errorf("ValidateSettings flagged a valid %s.%s=%q: %+v", section, key, good, got) + } + wrote = false + ctx2, _ := newMockCtx(t, withBackend(settingsBackend(&wrote))) + if err := alterSettings(ctx2, &ast.AlterSettingsStmt{ + Section: section, + ConfigName: "Default", + Properties: map[string]any{key: good}, + }); err != nil { + t.Errorf("executor rejected a valid %s.%s=%q: %v", section, key, good, err) + } + }) + } + } +} diff --git a/mdl/executor/validate_settings.go b/mdl/executor/validate_settings.go new file mode 100644 index 000000000..90f8c5c31 --- /dev/null +++ b/mdl/executor/validate_settings.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// settingsValueKind classifies the settings properties whose value the executor +// parses rather than storing verbatim. Anything unlisted is a string as written. +type settingsValueKind int + +const ( + settingsKindInt settingsValueKind = iota + settingsKindBool +) + +// typedSettingsKeys maps a lower-cased ALTER SETTINGS section to the properties +// whose value must parse as something other than a string. It mirrors the +// assignment switches in cmd_settings.go; TestTypedSettingsKeys_MatchExecutor +// fails if the two drift apart. +var typedSettingsKeys = map[string]map[string]settingsValueKind{ + "model": { + "BcryptCost": settingsKindInt, + "AllowUserMultipleSessions": settingsKindBool, + }, + "workflows": { + "DefaultTaskParallelism": settingsKindInt, + "WorkflowEngineParallelism": settingsKindInt, + }, + "configuration": { + "HttpPortNumber": settingsKindInt, + "ServerPortNumber": settingsKindInt, + }, +} + +// ValidateSettings reports ALTER SETTINGS values that will not parse as their +// property's type. The executor rejects them too, but reporting them at check time +// means a typo surfaces before the project is opened for writing (#805). +func ValidateSettings(stmt *ast.AlterSettingsStmt) []linter.Violation { + keys, ok := typedSettingsKeys[strings.ToLower(stmt.Section)] + if !ok { + return nil + } + return validateTypedSettings(keys, stmt.Properties, + "alter settings "+strings.ToLower(stmt.Section)) +} + +// ValidateCreateConfiguration reports the same for CREATE CONFIGURATION, which +// accepts the configuration properties directly. +func ValidateCreateConfiguration(stmt *ast.CreateConfigurationStmt) []linter.Violation { + return validateTypedSettings(typedSettingsKeys["configuration"], stmt.Properties, + fmt.Sprintf("create configuration '%s'", stmt.Name)) +} + +func validateTypedSettings(keys map[string]settingsValueKind, props map[string]any, what string) []linter.Violation { + if len(keys) == 0 || len(props) == 0 { + return nil + } + // Iterate the properties in a stable order: a map would make the diagnostics + // order (and so the check output) non-deterministic. + names := make([]string, 0, len(props)) + for key := range props { + names = append(names, key) + } + sort.Strings(names) + + loc := linter.Location{DocumentType: "settings", DocumentName: what} + var out []linter.Violation + for _, key := range names { + kind, ok := keys[key] + if !ok { + continue + } + valStr := settingsValueToString(props[key]) + switch kind { + case settingsKindInt: + if _, err := settingsInt(key, valStr); err != nil { + out = append(out, linter.Violation{ + RuleID: "MDL-SET01", + Severity: linter.SeverityError, + Location: loc, + Message: fmt.Sprintf("%s: %s must be an integer, got %q", what, key, valStr), + Suggestion: fmt.Sprintf("Use a whole number, quoted or not: `%s = 10` or `%s = '10'`.", key, key), + }) + } + case settingsKindBool: + if _, err := settingsBool(key, valStr); err != nil { + out = append(out, linter.Violation{ + RuleID: "MDL-SET02", + Severity: linter.SeverityError, + Location: loc, + Message: fmt.Sprintf("%s: %s must be true or false, got %q", what, key, valStr), + Suggestion: fmt.Sprintf("Use `%s = true` or `%s = false`.", key, key), + }) + } + } + } + return out +} From f1daae042ffbb1d4d3fc22c1e9a2ccc3ca0dfa11 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 30 Jul 2026 06:09:57 +0000 Subject: [PATCH 21/21] =?UTF-8?q?docs(show=5Fpage):=20document=20#56=20?= =?UTF-8?q?=E2=80=94=20describe=20omits=20redundant=20$currentObject=20arg?= =?UTF-8?q?=20(write=20is=20correct)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigation of FINDINGS #56 ("describe page omits a show_page action's arguments"), verified against mxbuild 11.12.1: A widget button's show_page stores FormSettings.ParameterMappings as an empty list [2] and Mendix infers the current-row object for each unmapped page parameter. Storing an explicit `Argument: "$currentObject"` mapping makes mxbuild report CE0115 "arguments do not match" — the original issue #296, re-confirmed here. So the empty-mapping write is REQUIRED for a building app. Consequence: `show_page X` and `show_page X($p = $currentObject)` serialize to identical BSON, so describe→drop→exec re-produces a byte-identical valid page — the round-trip is functionally lossless; only the redundant $currentObject annotation is not echoed. No writer change (a fix there reintroduces CE0115); clarified the serializer comment and added a bug-test documenting the verified behavior and the boundary (a non-$currentObject widget page arg needs a Studio-Pro WidgetValue reference to encode). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .../56-show-page-currentobject-arg.mdl | 53 +++++++++++++++++++ sdk/mpr/writer_widgets_action.go | 9 ++-- 2 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 mdl-examples/bug-tests/56-show-page-currentobject-arg.mdl diff --git a/mdl-examples/bug-tests/56-show-page-currentobject-arg.mdl b/mdl-examples/bug-tests/56-show-page-currentobject-arg.mdl new file mode 100644 index 000000000..b785a7421 --- /dev/null +++ b/mdl-examples/bug-tests/56-show-page-currentobject-arg.mdl @@ -0,0 +1,53 @@ +-- ============================================================================ +-- FINDINGS #56: `describe page` omits a show_page action's `$currentObject` +-- argument — investigated; the write is correct and the round-trip is +-- functionally lossless. +-- ============================================================================ +-- +-- Symptom: `describe page` renders an action button whose action is +-- show_page Mod.EditEntry($Entry = $currentObject) +-- as just `Action: show_page Mod.EditEntry` — the `($Entry = $currentObject)` +-- argument is not shown. +-- +-- Investigation (verified against mxbuild 11.12.1): +-- * A widget button's `show_page` stores its FormSettings.ParameterMappings as +-- an EMPTY list `[2]`; Mendix INFERS the current-row object for each unmapped +-- page parameter (a list/dataview button's current object is represented by +-- an inferred WidgetValue, not an Argument expression). +-- * Storing an explicit `Argument: "$currentObject"` mapping makes Studio Pro / +-- mxbuild report CE0115 "arguments … do not match … need to be refreshed" +-- (this is the original issue #296, re-confirmed here). So the empty-mapping +-- write is REQUIRED for a building app — it is not a bug. +-- * Because of this, `show_page X` (no args) and `show_page X($p = $currentObject)` +-- serialize to IDENTICAL BSON. A describe → drop → exec round-trip therefore +-- re-produces a byte-identical, valid page; only the redundant `$currentObject` +-- annotation is not echoed back. The round-trip is functionally lossless. +-- +-- Known boundary (NOT covered here): expressing a NON-$currentObject page-parameter +-- argument on a widget button (e.g. binding a page param through) needs the +-- Studio-Pro `WidgetValue`-vs-`Argument` representation, which requires a +-- known-good Studio Pro reference to encode safely (see .claude/skills/debug-bson.md). +-- +-- The MICROFLOW-side `show page` (in a microflow, not a widget) DOES round-trip +-- its arguments — see 02-microflow-examples — because a microflow has no +-- current-object inference and stores an explicit Argument. +-- +-- This script builds clean (`mx check` → 0 errors); the show_page button binds +-- $Entry to the list's current row by inference. +-- ============================================================================ + +create entity MyFirstModule.Entry ( Label: String ); +/ +create or replace page MyFirstModule.EditEntry +( Title: 'Edit', Layout: Atlas_Core.Atlas_Default, Params: { $Entry: MyFirstModule.Entry } ) +{ + dataview dv (datasource: $Entry) { dynamictext t (content: 'Edit') } +} +/ +create or replace page MyFirstModule.EntryList ( Title: 'List', Layout: Atlas_Core.Atlas_Default ) +{ + listview lv (datasource: database MyFirstModule.Entry) { + actionbutton weEdit (caption: 'Edit', action: show_page MyFirstModule.EditEntry($Entry = $currentObject)) + } +} +/ diff --git a/sdk/mpr/writer_widgets_action.go b/sdk/mpr/writer_widgets_action.go index b9daed8c6..6f9b24b03 100644 --- a/sdk/mpr/writer_widgets_action.go +++ b/sdk/mpr/writer_widgets_action.go @@ -82,9 +82,12 @@ func serializeClientAction(action pages.ClientAction) bson.D { case *pages.PageClientAction: // Studio Pro stores ParameterMappings as an empty initialized array [2] and // infers $currentObject from the enclosing widget context (DataGrid, DataView, etc.). - // Storing explicit inline Forms$PageParameterMapping objects uses an invalid type - // indicator (len instead of 2/3), causing Studio Pro to read 0 mappings and - // report CE0115 "parameter not passed" even when mappings are present (issue #296). + // Storing explicit inline Forms$PageParameterMapping objects with an Argument of + // "$currentObject" makes Studio Pro report CE0115 "parameters do not match" — a + // widget's current-row object is represented by an inferred WidgetValue, not an + // Argument expression (issue #296; re-confirmed against mxbuild 11.12.1 for + // FINDINGS #56 — DESCRIBE recovers the implicit $currentObject instead, see + // renderClientActionMDL). formSettings := bson.D{ {Key: "$ID", Value: idToBsonBinary(generateUUID())}, {Key: "$Type", Value: "Forms$FormSettings"},