From 5abfc8dce86824bb075bd575819aa59dafacb662 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:39:11 +0000 Subject: [PATCH 01/13] fix(theme): name seeded font families theme create does not vendor Part of #944 (item 4). Co-Authored-By: Claude Opus 5.5 --- .../skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + CHANGELOG.md | 1 + cmd/mxcli/cmd_theme.go | 10 ++++ cmd/mxcli/theme/create.go | 8 +++ cmd/mxcli/theme/create_seeded.go | 53 ++++++++++++++++++- cmd/mxcli/theme/create_seeded_test.go | 51 ++++++++++++++++++ 6 files changed, 123 insertions(+), 1 deletion(-) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index a5cc6021f8..d8de3876d3 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -149,3 +149,4 @@ {"date": "2026-10-02", "area": "cmd/mxcli", "symptom": "`run --local --watch` on Mendix 11.13: a page added while the loop runs is never bundled — the apply reports success (often 'applied via reload'), but opening the page 404s on dist/pages/..js and the page stays blank; restarting the loop fixes it", "cause": "mxbuild's tools/node/rollup-plugin-mendix-pages.mjs (byte-identical in 11.13.0 and 11.14.0) globs web/pages only at bundler start: watchChange compares path.relative(cwd, id) against PAGES_FOLDER=\"./pages\" + \"/\", a './' prefix relative() never yields, so shouldRefreshPageFiles never flips. mxcli could not see it: ensureClientServed probes index.js and its static imports, and pages are dynamic imports", "fix": "missingPageChunks compares web/pages/**/*.js against web/dist/pages/**/*.js (shape-gated: needs web/pages and web/dist/index.js, so 11.14 prebuilt and classic are no-ops); under --watch watchAndApply restarts the WebClientWatcher (a fresh rollup run re-globs) and reports one line; ensureClientServed does a one-shot BuildWebClient for the same condition. RunLocal stops whichever watcher is current at exit", "insight": "A stand-alone rollup watch repro (pages/A.js, then add pages/B.js) isolates it in seconds: the 'change pages/B.js create' event fires but the next bundle still emits only A; a fresh watcher, or the plugin with PAGES_FOLDER=\"pages\", emits both. Restart rather than one-shot: the stale watcher would also never rebuild the new page when it is edited later. A structural change (navigation) clears web/dist and the old index.js fallback re-bundles everything, masking the bug; a page-only add applied via reload is the reproducing case. E2E 11.13: pre-change binary 404 on the new chunk; fixed binary 200 + page text, and a follow-up edit of the same page re-bundled incrementally with no restart (control)", "file": "cmd/mxcli/docker/webclient_pages.go (missingPageChunks, recoverMissingPages); cmd/mxcli/docker/runlocal.go (watchAndApply, ensureClientServed, RunLocal watcher defer)", "test": "cmd/mxcli/docker/webclient_pages_test.go"} {"date": "2026-10-02", "area": "cmd/mxcli", "symptom": "A `.test.mdl` starting with `mdl 1;` loses its first test: `mxcli test --list` finds 41 of 42 with no message; `mxcli check` on it reads the bodies as mdl 0 (MDL-V1-SLASH / MDL-V1-LIMIT1 warnings fire under a file that says mdl 1); the generated runner scripts end every flow with `/`, which mdl 1 refuses; and `fmt --upgrade --header` adds no header to a test file", "cause": "Nothing in the test format read the header. It was body text in the first `/`-chunk, so that chunk's `/** @test */` was no longer a LEADING doc comment (scanDocComments) and parseMDLTests skipped it silently; CheckSource rendered only the bodies (dropping the header line) and put `END; /` on separators; GenerateTestRunner/GenerateTestFlows always wrote `/` and `create or replace`", "fix": "takeLanguageHeader reads the header with langver.HeaderSpan (the grammar's own rule: first token after trivia) and blanks it in place so lines/columns hold; TestCase carries Version + HeaderLine (markdown: per mdl-test block); CheckSource renders the header on its line and closes wrappers with `END;` (no `/`, any version); the generators write the header, `create or modify` and no `/` under mdl 1; parseTestFiles refuses a suite mixing versions (one suite = one script = one header); UpgradeSource honours AddHeader and writes the header at file top / inside each mdl-test block; UpgradeSource maps the upgraded rendering back with a line diff (difflib opcodes, every changed region must be body lines) instead of an index walk, because the MDL-V1-ESCAPE rewrite of `\\n` writes a real line break and adds a line; writeBodyLines no longer indents a body line that starts inside a string literal", "insight": "A header is not just a flag to pass along — in a format that is NOT parsed as one script, every consumer that splits the text has to take it out first, or it becomes content in whichever chunk it lands in. The silent drop came from the same 'leading doc comment' rule as #927; a per-test Version also forced the question 'what is a suite's version', which only refusal answers safely. The runtime run was what found the last defect: unit tests and `mx check` were green while an upgraded test asserting length 3 saw 5 — the generators indented the continuation line of a multi-line string literal, so the indent became part of the value. Compare an mdl 0 file, its fmt --upgrade output, and a headerless control in the running app (both runners)", "issue": "ako/mxcli#847", "file": "cmd/mxcli/testrunner/parser.go (takeLanguageHeader, suiteLanguageVersion); check_source.go; generator.go; generator_endpoint.go; upgrade_source.go; mdl/langver/langver.go (HeaderSpan)", "test": "cmd/mxcli/testrunner/language_header_test.go; e2e: mxcli test --local on a fresh 11.13.0 app, endpoint and --legacy-runner"} {"date": "2026-10-02", "area": "cmd/mxcli/check", "symptom": "`mxcli check x.test.mdl -p app.mpr` reports `module not found: MxTest` once per test and exits 1 on a file `mxcli test` runs green — a test file can never pass check, and a real missing reference in a body is hidden behind it", "cause": "CheckSource wraps each test body in a `MxTest.Check_*` microflow (the runner's module), and the reference pass resolved that module against the project; the runner creates MxTest as the first statement of every generated script and drops it afterwards, so the project never has it", "fix": "testrunner.WithRunnerModule appends `create module if not exists MxTest` to the program used for the reference and project-conflict pass of a test file (appended so `statement N` still numbers the tests; definitions are collected program-wide; `if not exists` keeps a user's own MxTest from reading as a conflict)", "insight": "Seed the module the way the runner does, rather than exempting the file: the control (a body with a real missing entity) must still fail, and before the fix it showed only the MxTest error — the false positive was also masking true ones", "issue": "ako/mxcli#677", "file": "cmd/mxcli/testrunner/check_source.go (WithRunnerModule); cmd/mxcli/cmd_check.go", "test": "cmd/mxcli/check_test_file_refs_test.go"} +{"date": "2026-10-03", "area": "cmd/mxcli/theme", "symptom": "`theme create acme --from design.css` with `--mxt-font: \"Inter\", system-ui, sans-serif` prints nothing about Inter; the theme ships no woff2 and no @font-face for it and renders in the fallback font wherever Inter is not installed", "cause": "planFonts only decided which VENDORED families to drop; a seeded family outside the vendored set was never looked at, so the silent outcome was the default", "fix": "unvendoredSeededFamilies takes the primary (first) family of each seeded font stack, skips generic families and var() and the families the base partial loads, and CreateResult.UnvendoredFonts carries them to cmd_theme.go, which prints a note per family naming mxcli-fonts/ and the partial", "insight": "Only the first family of a stack is the design's choice; flagging the fallbacks (Helvetica, Arial) would make the note noise. The controls are a vendored family (IBM Plex Mono) and a generic stack, which must stay silent", "issue": "ako/mxcli#944", "file": "cmd/mxcli/theme/create_seeded.go (unvendoredSeededFamilies, planFonts); cmd/mxcli/cmd_theme.go", "test": "cmd/mxcli/theme/create_seeded_test.go (TestCreate_NamesSeededFontsItDoesNotVendor)"} diff --git a/CHANGELOG.md b/CHANGELOG.md index f9ac22a378..0c10b7d35a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`theme create --from` names a seeded font family it does not ship** (ako/mxcli#944) — a design whose `--mxt-font`, `--mxt-font-heading` or `--mxt-font-mono` is led by a family the base theme does not vendor (Inter, say) got no `@font-face` and no file, and nothing was printed, so the theme rendered in a fallback font wherever the family was not installed. `theme create` now prints a note per such family, naming where to add its woff2 files and `@font-face`. A stack led by a generic family (`system-ui`, `sans-serif`, …) is not named. - **`page.title` and `catalog.pages.Title` are the project's default language** (mendixlabs/mxcli#1262) — the catalog took whichever translation of a page title a Go map range met first, so a lint rule reading `page.title` changed output between catalog rebuilds of an unchanged project. The title is now the project's default language, else en_US, else the lowest-sorted non-empty language; every translation stays available in `catalog.strings` (`StringContext = 'Forms$Page.Title'`). The catalog schema version moves to 17, so a cached catalog is rebuilt. - **Lint rules that read full-catalog data no longer pass silently** — `widgets()`, `xpath_expressions()`, `activities_for()`, `permissions()`, `permissions_for()` and a page's or snippet's `widget_count` read data only `refresh catalog full` writes, but only `refs_to` / `refs_from` raised the build depth, so unless another rule in the set happened to call `refs_to` the build stayed fast, they returned `[]` / `0` and the rule reported nothing (`widgets()`: 0 rows vs 46 on PedApp). They are now auto-detected like `refs_to`. The built-in MPR005, MPR006 and MPR012 read widgets the same way and were silent on a project without `.claude/lint-rules/` (an empty container went unreported); they now request the full build, so `mxcli lint` always builds a full catalog (measured on TestApp: 2.3 s -> 3.1 s for the build). diff --git a/cmd/mxcli/cmd_theme.go b/cmd/mxcli/cmd_theme.go index 2af9652150..f8d2ca13e0 100644 --- a/cmd/mxcli/cmd_theme.go +++ b/cmd/mxcli/cmd_theme.go @@ -247,6 +247,16 @@ Examples: res.Tokens.Count(), res.Tokens.Source, len(res.Tokens.Base), len(res.Tokens.Dark), len(res.Tokens.Light)) } + // A seeded family mxcli does not vendor gets no @font-face and no file, + // so it renders only on machines that happen to have it (#944). + for _, fam := range res.UnvendoredFonts { + fmt.Printf("\nNote: font family %q is not vendored: the theme names it but ships no file "+ + "and no @font-face, so it renders only where installed. To ship it, add its woff2 "+ + "files under %s/files/theme/web/mxcli-fonts/ and an @font-face for them in "+ + "%s/files/theme/web/_mxcli-%s.scss.\n", + fam, filepath.ToSlash(root), filepath.ToSlash(root), res.Name) + } + if !dryRun { fmt.Printf("\nEdit the palette, then apply it:\n"+ " mxcli theme apply %s -p \n", res.Name) diff --git a/cmd/mxcli/theme/create.go b/cmd/mxcli/theme/create.go index b379b6e72c..48a2b3914f 100644 --- a/cmd/mxcli/theme/create.go +++ b/cmd/mxcli/theme/create.go @@ -90,6 +90,7 @@ func Create(projectDir, name string, opts CreateOptions) (*CreateResult, error) if err := rewrite.planFonts(src, root, tokens); err != nil { return nil, err } + res.UnvendoredFonts = rewrite.unvendoredFonts walkErr := fs.WalkDir(src.fsys, root, func(p string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { @@ -234,6 +235,9 @@ type CreateResult struct { Dir string Files []FileResult Tokens *Tokens + // UnvendoredFonts are the seeded font families the theme names but does + // not ship: no woff2, no @font-face. They render only where installed. + UnvendoredFonts []string } // rewriter carries the renames that turn a copy of one theme into another: @@ -254,6 +258,10 @@ type rewriter struct { // keptAnyFont records whether any @font-face survived, which decides // whether the mxcli-fonts/ directory and its licence are still shipped. keptAnyFont bool + // unvendoredFonts are the seeded primary families no @font-face in the + // base theme loads (#944): the theme names them, ships no file for them, + // and so renders them only where the font happens to be installed. + unvendoredFonts []string } func newRewriter(base *Theme, newName, newTitle string, tokens *Tokens) *rewriter { diff --git a/cmd/mxcli/theme/create_seeded.go b/cmd/mxcli/theme/create_seeded.go index c21d92c71b..3f500f6389 100644 --- a/cmd/mxcli/theme/create_seeded.go +++ b/cmd/mxcli/theme/create_seeded.go @@ -129,7 +129,55 @@ func unusedVendoredFamilies(scss string, tokens *Tokens) []string { return unused } +// genericFontFamilies are the CSS generic families and system-font aliases: a +// stack led by one of these asks for whatever the platform has, so there is no +// file to ship and nothing to warn about. +var genericFontFamilies = map[string]bool{ + "serif": true, "sans-serif": true, "monospace": true, "cursive": true, + "fantasy": true, "math": true, "emoji": true, "fangsong": true, + "system-ui": true, "ui-serif": true, "ui-sans-serif": true, + "ui-monospace": true, "ui-rounded": true, + "-apple-system": true, "blinkmacsystemfont": true, + "inherit": true, "initial": true, "unset": true, "revert": true, +} + +// unvendoredSeededFamilies returns the primary family of each seeded font stack +// that the base partial does not load with @font-face (#944). +// +// Only the first family counts: it is the one the design chose, and the rest +// of the stack is the fallback that by definition renders "where installed". A +// stack led by a generic family or a var() reference names no font file to +// ship. A family is vendored when the partial loads it, compared without case. +func unvendoredSeededFamilies(scss string, tokens *Tokens) []string { + if tokens == nil { + return nil + } + vendored := map[string]bool{} + for _, fam := range vendoredFamilies(scss) { + vendored[strings.ToLower(fam)] = true + } + var out []string + seen := map[string]bool{} + for _, name := range []string{"--mxt-font", "--mxt-font-heading", "--mxt-font-mono"} { + v, ok := tokens.Base[name] + if !ok { + continue + } + first := strings.TrimSpace(strings.SplitN(v, ",", 2)[0]) + first = strings.Trim(first, `"'`) + key := strings.ToLower(first) + if first == "" || strings.HasPrefix(key, "var(") || genericFontFamilies[key] || + vendored[key] || seen[key] { + continue + } + seen[key] = true + out = append(out, first) + } + return out +} + // dropFontFaces removes the @font-face blocks for the named families, and the + // section comment when nothing is left to explain. func dropFontFaces(scss string, families []string) string { for _, fam := range families { @@ -219,10 +267,13 @@ func (r *rewriter) planFonts(src source, root string, tokens *Tokens) error { if err != nil { // A base theme with no such partial simply has no vendored fonts to // reason about. That is not an error — it is a theme that already - // relies on system fonts. + // relies on system fonts. Every seeded family is then unvendored. + r.unvendoredFonts = unvendoredSeededFamilies("", tokens) return nil } scss := string(body) + r.unvendoredFonts = unvendoredSeededFamilies(scss, tokens) + r.droppedFonts = unusedVendoredFamilies(scss, tokens) r.keptAnyFont = len(r.droppedFonts) < len(vendoredFamilies(scss)) return nil diff --git a/cmd/mxcli/theme/create_seeded_test.go b/cmd/mxcli/theme/create_seeded_test.go index 9e45913258..abebb331c9 100644 --- a/cmd/mxcli/theme/create_seeded_test.go +++ b/cmd/mxcli/theme/create_seeded_test.go @@ -313,3 +313,54 @@ func TestDropFontFacesKeepsBracesBalanced(t *testing.T) { t.Fatal("no shipped partial declares a vendored family — the glob no longer finds the assets") } } + +// A seeded family mxcli does not vendor (Inter) got no @font-face and no file, +// and `theme create --from` said nothing (#944), so the theme rendered in a +// fallback font everywhere Inter was not installed. Create must name each such +// family; the controls are a vendored family and a generic stack, which ship or +// need nothing and must not be named. +func TestCreate_NamesSeededFontsItDoesNotVendor(t *testing.T) { + tests := map[string]struct { + css string + want []string + }{ + "an unvendored body font is named": { + css: `:root { --mxt-font: "Inter", system-ui, sans-serif; }`, + want: []string{"Inter"}, + }, + "each unvendored stack is named once": { + css: `:root { + --mxt-font: 'Inter', sans-serif; + --mxt-font-heading: Inter, sans-serif; + --mxt-font-mono: "JetBrains Mono", monospace; + }`, + want: []string{"Inter", "JetBrains Mono"}, + }, + "control: a vendored family is not named": { + css: `:root { --mxt-font-mono: "IBM Plex Mono", ui-monospace, monospace; }`, + want: nil, + }, + "control: a generic stack is not named": { + css: `:root { --mxt-font: system-ui, -apple-system, sans-serif; }`, + want: nil, + }, + "control: no fonts seeded": { + css: `:root { --mxt-brand: #10069F; }`, + want: nil, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + dir := newProject(t) + design := filepath.Join(dir, "design.css") + write(t, design, tc.css) + res, err := Create(dir, "probe", CreateOptions{From: design}) + if err != nil { + t.Fatal(err) + } + if strings.Join(res.UnvendoredFonts, ",") != strings.Join(tc.want, ",") { + t.Errorf("UnvendoredFonts = %v, want %v", res.UnvendoredFonts, tc.want) + } + }) + } +} From d1ede6716f0000c2e01832036539e283babba33e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:43:08 +0000 Subject: [PATCH 02/13] fix(executor): drop names the module-role grants it removes drop microflow/nanoflow/page print the removed roles, whether a create carries them (same script or session for flows, never for a page) and the grant that restores them. The splice refusal and the flow skills say to drop and create in one script. Part of #944 (item 3). Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../skills/mendix/write-microflows/SKILL.md | 6 ++ .../skills/mendix/write-nanoflows/SKILL.md | 7 +- CHANGELOG.md | 1 + mdl/executor/cmd_microflows_drop.go | 2 + mdl/executor/cmd_nanoflows_drop.go | 2 + mdl/executor/cmd_pages_builder.go | 2 + mdl/executor/drop_grants_note.go | 40 +++++++ mdl/executor/drop_grants_note_test.go | 102 ++++++++++++++++++ mdl/executor/flow_verdict.go | 4 +- mdl/executor/flow_verdict_test.go | 6 ++ 11 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 mdl/executor/drop_grants_note.go create mode 100644 mdl/executor/drop_grants_note_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index c672869457..32ceb70628 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -836,3 +836,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: TestApp WorkflowCommons snippets break exec — `combobox … (Attribute: TimeFrame)` \"has no entity to bind against\", image `Visible: CompletionType in (…)` \"place the widget inside a data container\"; widgets sit directly in a snippet with a parameter, no data view", "cause": "Studio Pro binds a widget outside every data container to a page/snippet PARAMETER: AttributeRef (or ConditionalVisibilitySettings.Attribute, or a combo box IndirectEntityRef) beside SourceVariable Forms$PageVariable {SnippetParameter|PageParameter: name, Widget: \"\"}. describe printed the attribute bare and exec had no spelling for the source, so it refused (or, before describe/pluggable Visible were fixed, silently wrote no binding)", "file": "`mdl/executor/cmd_pages_parameter_binding.go`, `cmd_pages_builder_v3.go` (resolveInputBinding/parameterVariable), `widget_engine.go` (Attribute/Association mappings), `cmd_pages_builder_visible_when.go`, describe in `cmd_pages_describe_parse.go`/`cmd_pages_describe_pluggable.go`, writer `widgetobj.SetSourceVariable`, `conditionalVisibilityToGen`", "insight": "A newly fixed describe gap can surface as an exec refusal the allowlist never expected: the refusal was right for the bare spelling and the cure is a spelling for the stored source (`$Param.Attr`, `$Param.Module.Assoc`, `Visible: $Param.Attr in (…)`), not a looser guard. Survey SourceVariable slot combinations (W/P/S/L) across the fixture first: TestApp had --S- on built-in inputs, pluggable values and 76 visibility settings, all unspelled", "refs": ["#721", "#826"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "describe prints a pluggable image's `Visible: Attr in (…)` (and expression Visible/Editable) twice", "cause": "the image branch appended appendConditionalProps and then appendAppearanceProps, which appends the same conditional settings", "file": "`mdl/executor/cmd_pages_describe_output.go` (image branch)", "insight": "appendAppearanceProps already owns visibility/editability; a branch that also appends them duplicates the key — grep for both on one widget kind", "refs": ["#721"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: describe → exec of TestApp's WorkflowCommons.UserTask_Assign fails: \"widget `grid8` (datagrid) cannot have its visibility set: its widget package declares no Visibility system property\"", "cause": "MDL-WIDGET41 gated Visible: on the package declaring , measured only from which packages declare it — but Studio Pro stores ConditionalVisibilitySettings on a Datagrid whose Type declares no Visibility property, and mxbuild 11.14 accepts static and conditional visibility there (0 errors)", "file": "`mdl/executor/pluggable_system_props.go` (`undeclaredSystemProps`)", "insight": "A declared system property is evidence of where a setting is shown, not of whether it exists; a refusal must be measured against what Studio Pro actually stores. Run the roundtrip harness over Studio Pro-authored pages before adding a refusal on stored shapes. Editability stays gated", "refs": []} +{"date": "2026-10-03", "area": "mdl/executor/drop", "symptom": "`drop microflow M.F;` in one `mxcli exec` run and `create microflow M.F …` in the next leaves M.F with no module-role grants (CE0106 on the pages calling it); the drop printed only \"Dropped microflow: M.F\"", "cause": "the grants carry only through the session cache (rememberDroppedMicroflow / consumeDroppedMicroflow), which a later process does not have; nothing told the user the carry was session-scoped. `drop page` never remembers its AllowedRoles at all", "fix": "writeDroppedGrantsNote (mdl/executor/drop_grants_note.go), called from execDropMicroflow / execDropNanoflow / execDropPage, prints the removed roles, whether a create carries them (same script or session for flows; never for a page), and the `grant` that restores them; flowRefusal's rebuild advice says drop + create in the same script", "insight": "A carry that lives in a session cache is invisible at the statement that creates it; the place to say so is the drop, which is the last moment the roles are known. Snippets have no access roles, so they need nothing", "issue": "ako/mxcli#944", "file": "mdl/executor/drop_grants_note.go; cmd_microflows_drop.go; cmd_nanoflows_drop.go; cmd_pages_builder.go; flow_verdict.go", "test": "mdl/executor/drop_grants_note_test.go; flow_verdict_test.go (TestFlowRefusalNamesTheFlowAndTheReason)"} diff --git a/.claude/skills/mendix/write-microflows/SKILL.md b/.claude/skills/mendix/write-microflows/SKILL.md index 3a5e49e872..d2c8ce53ca 100644 --- a/.claude/skills/mendix/write-microflows/SKILL.md +++ b/.claude/skills/mendix/write-microflows/SKILL.md @@ -46,6 +46,12 @@ Choose the mode by who owns the microflow ([choose-edit-mode](../choose-edit-mod patched in place (a move keeps the node's flows). A redrawn `@anchor`/`@curve`, loop body, error handler or other `return` added/taken away rebuilds under mdl 0 (`MDL-V1-REBUILD`: IDs renumbered, merges and curves lost) and is refused under `mdl 1;`. **To change a loop body, `alter … replace` the whole loop** — neither mode edits inside one ([pitfalls](reference/pitfalls.md#11-changing-something-inside-a-loop-body)). +- **Rebuilding deliberately: `drop microflow X;` and `create microflow X …` in ONE script.** + The drop's module-role grants (and the unit's ID and folder) carry only to a create later in + the same script or REPL session. A drop in one run and a create in the next loses every + `grant execute` (CE0106 on the pages that call it); `drop` prints the roles it removed and + the `grant` to restore them. A `drop page` never carries its view grants. + ## When to Use a Microflow vs a Nanoflow diff --git a/.claude/skills/mendix/write-nanoflows/SKILL.md b/.claude/skills/mendix/write-nanoflows/SKILL.md index 283c7bdfd3..e5b9bb28d9 100644 --- a/.claude/skills/mendix/write-nanoflows/SKILL.md +++ b/.claude/skills/mendix/write-nanoflows/SKILL.md @@ -38,7 +38,12 @@ Choose the mode by who owns the nanoflow ([choose-edit-mode](../choose-edit-mode rebuilds the whole nanoflow under mdl 0 (warning `MDL-V1-REBUILD`: element IDs renumbered, merges removed, curves reset) and is refused under `mdl 1;`. -- **Changing something inside a loop body** (either owner): neither `create or modify` +- **Rebuilding deliberately: `drop nanoflow X;` and `create nanoflow X …` in ONE script.** + The drop's module-role grants (and the unit's ID and folder) carry only to a create later in + the same script or REPL session; a drop in one run and a create in the next loses every + `grant execute`. `drop` prints the roles it removed and the `grant` to restore them. +- **Changing something inside a loop body** (either owner) +: neither `create or modify` (under `mdl 1;`) nor an `alter` aimed at an activity in the loop can make it — `alter` does not splice inside a loop. Replace the **whole loop**, addressed by its handle, with the body as it should be (`replace loop $Item in $Items with begin loop $Item in $Items diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c10b7d35a..ce361291af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`drop microflow`, `drop nanoflow` and `drop page` name the module-role grants they remove** (ako/mxcli#944) — a drop printed only "Dropped microflow", so a drop in one run and a create in the next silently lost every `grant execute` (CapTrack hit CE0106). The drop now lists the roles, says a create later in the same script or session carries them (a microflow or nanoflow; a page never carries), and prints the `grant` that restores them. The `create or modify` splice refusal and the write-microflows / write-nanoflows skills say to do the drop and the create in one script. - **`theme create --from` names a seeded font family it does not ship** (ako/mxcli#944) — a design whose `--mxt-font`, `--mxt-font-heading` or `--mxt-font-mono` is led by a family the base theme does not vendor (Inter, say) got no `@font-face` and no file, and nothing was printed, so the theme rendered in a fallback font wherever the family was not installed. `theme create` now prints a note per such family, naming where to add its woff2 files and `@font-face`. A stack led by a generic family (`system-ui`, `sans-serif`, …) is not named. - **`page.title` and `catalog.pages.Title` are the project's default language** (mendixlabs/mxcli#1262) — the catalog took whichever translation of a page title a Go map range met first, so a lint rule reading `page.title` changed output between catalog rebuilds of an unchanged project. The title is now the project's default language, else en_US, else the lowest-sorted non-empty language; every translation stays available in `catalog.strings` (`StringContext = 'Forms$Page.Title'`). The catalog schema version moves to 17, so a cached catalog is rebuilt. diff --git a/mdl/executor/cmd_microflows_drop.go b/mdl/executor/cmd_microflows_drop.go index 304af10b4e..7fd804aeee 100644 --- a/mdl/executor/cmd_microflows_drop.go +++ b/mdl/executor/cmd_microflows_drop.go @@ -48,6 +48,8 @@ func execDropMicroflow(ctx *ExecContext, s *ast.DropMicroflowStmt) error { } invalidateHierarchy(ctx) fmt.Fprintf(ctx.Output, "Dropped microflow: %s.%s\n", s.Name.Module, s.Name.Name) + writeDroppedGrantsNote(ctx.Output, "microflow", "execute", qualifiedName, mf.AllowedModuleRoles, true) + return nil } } diff --git a/mdl/executor/cmd_nanoflows_drop.go b/mdl/executor/cmd_nanoflows_drop.go index 93a01d2be1..283000f5cd 100644 --- a/mdl/executor/cmd_nanoflows_drop.go +++ b/mdl/executor/cmd_nanoflows_drop.go @@ -43,6 +43,8 @@ func execDropNanoflow(ctx *ExecContext, s *ast.DropNanoflowStmt) error { } invalidateHierarchy(ctx) fmt.Fprintf(ctx.Output, "Dropped nanoflow: %s.%s\n", s.Name.Module, s.Name.Name) + writeDroppedGrantsNote(ctx.Output, "nanoflow", "execute", qualifiedName, nf.AllowedModuleRoles, true) + return nil } } diff --git a/mdl/executor/cmd_pages_builder.go b/mdl/executor/cmd_pages_builder.go index 2554049348..363fa5fb62 100644 --- a/mdl/executor/cmd_pages_builder.go +++ b/mdl/executor/cmd_pages_builder.go @@ -429,6 +429,8 @@ func execDropPage(ctx *ExecContext, s *ast.DropPageStmt) error { return mdlerrors.NewBackend("delete page", err) } fmt.Fprintf(ctx.Output, "Dropped page %s\n", s.Name.String()) + writeDroppedGrantsNote(ctx.Output, "page", "view", s.Name.String(), p.AllowedRoles, false) + return nil } } diff --git a/mdl/executor/drop_grants_note.go b/mdl/executor/drop_grants_note.go new file mode 100644 index 0000000000..aeaf806acf --- /dev/null +++ b/mdl/executor/drop_grants_note.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "io" + "strings" + + "github.com/mendixlabs/mxcli/model" +) + +// writeDroppedGrantsNote says which module-role grants a drop removed, and +// whether a later create gets them back (ako/mxcli#944). +// +// A dropped microflow or nanoflow is remembered in the session cache +// (rememberDroppedMicroflow), so a create of the same name later in the SAME +// script or REPL session carries the grants. A create in a later run starts +// from nothing — CapTrack's apply.sh dropped in one run and created in the +// next, and its pages lost access (CE0106) with only "Dropped microflow" to go +// on. A page is never remembered, so a create after its drop never carries. +// +// carries reports whether this document kind is remembered at all. grantVerb +// and kind spell the re-grant statement (`grant execute on microflow …`). +func writeDroppedGrantsNote(w io.Writer, kind, grantVerb, qualifiedName string, roles []model.ID, carries bool) { + if len(roles) == 0 { + return + } + names := strings.Join(documentRoleStrings(roles), ", ") + if carries { + fmt.Fprintf(w, " Removed its %s grants to %s. A create of %s later in this script or session "+ + "carries them; a create in a later run does not — do the drop and the create in one script, "+ + "or re-grant: grant %s on %s %s to %s;\n", + grantVerb, names, qualifiedName, grantVerb, kind, qualifiedName, names) + return + } + fmt.Fprintf(w, " Removed its %s grants to %s. A create of %s does not carry them; "+ + "re-grant: grant %s on %s %s to %s;\n", + grantVerb, names, qualifiedName, grantVerb, kind, qualifiedName, names) +} diff --git a/mdl/executor/drop_grants_note_test.go b/mdl/executor/drop_grants_note_test.go new file mode 100644 index 0000000000..b1411e429c --- /dev/null +++ b/mdl/executor/drop_grants_note_test.go @@ -0,0 +1,102 @@ +// 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/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// `drop microflow` printed only "Dropped microflow", so a drop in one run and a +// create in the next silently lost the module-role grants (CapTrack's apply.sh, +// CE0106; ako/mxcli#944). The drop must name the roles it removes and say that +// only a create in the same script or session carries them. The control is a +// flow with no grants, which has nothing to say. +func TestDropMicroflow_NamesTheGrantsItRemoves(t *testing.T) { + for _, tc := range []struct { + name string + roles []model.ID + want []string + }{ + {"granted", []model.ID{"MyModule.User", "MyModule.Admin"}, []string{ + "Removed its execute grants to MyModule.User, MyModule.Admin", + "later in this script or session carries them", + "grant execute on microflow MyModule.DoSomething to MyModule.User, MyModule.Admin;", + }}, + {"control: no grants", nil, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("MyModule") + mf := mkMicroflow(mod.ID, "DoSomething") + mf.AllowedModuleRoles = tc.roles + h := mkHierarchy(mod) + withContainer(h, mf.ContainerID, mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + DeleteMicroflowFunc: func(model.ID) error { return nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + assertNoError(t, execDropMicroflow(ctx, &ast.DropMicroflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "DoSomething"}, + })) + out := buf.String() + for _, w := range tc.want { + assertContainsStr(t, out, w) + } + if tc.want == nil && strings.Contains(out, "Removed") { + t.Errorf("a flow with no grants reported removed grants:\n%s", out) + } + }) + } +} + +func TestDropNanoflow_NamesTheGrantsItRemoves(t *testing.T) { + mod := mkModule("MyModule") + nf := mkNanoflow(mod.ID, "DoIt") + nf.AllowedModuleRoles = []model.ID{"MyModule.User"} + h := mkHierarchy(mod) + withContainer(h, nf.ContainerID, mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { return []*microflows.Nanoflow{nf}, nil }, + DeleteNanoflowFunc: func(model.ID) error { return nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + assertNoError(t, execDropNanoflow(ctx, &ast.DropNanoflowStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "DoIt"}, + })) + assertContainsStr(t, buf.String(), "Removed its execute grants to MyModule.User") + assertContainsStr(t, buf.String(), "grant execute on nanoflow MyModule.DoIt to MyModule.User;") +} + +// A page is not remembered across its drop, so no create carries its grants; +// the note must not promise that one does. +func TestDropPage_NamesTheGrantsItRemoves(t *testing.T) { + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "HomePage") + pg.AllowedRoles = []model.ID{"MyModule.User"} + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + DeletePageFunc: func(model.ID) error { return nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + assertNoError(t, execDropPage(ctx, &ast.DropPageStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "HomePage"}, + })) + out := buf.String() + assertContainsStr(t, out, "Removed its view grants to MyModule.User. A create of MyModule.HomePage does not carry them") + assertContainsStr(t, out, "grant view on page MyModule.HomePage to MyModule.User;") + if strings.Contains(out, "carries them") { + t.Errorf("a page drop promised a carry that does not happen:\n%s", out) + } +} diff --git a/mdl/executor/flow_verdict.go b/mdl/executor/flow_verdict.go index 22c100c033..8976b5f5ed 100644 --- a/mdl/executor/flow_verdict.go +++ b/mdl/executor/flow_verdict.go @@ -74,8 +74,10 @@ func flowRefusal(d *flowDecl, why error) error { "create or modify %s %s: this change cannot be spliced into the stored flow: %v. "+ "Nothing was written: rebuilding the whole flow instead would reset what Studio Pro drew "+ "(curves, merges, element IDs). Change activities with `alter %s %s { … }`; "+ - "to rebuild the flow deliberately, drop the %s and create it", + "to rebuild the flow deliberately, drop the %s and create it in the same script "+ + "(the drop's module-role grants carry to a create in the same script or session, not to one in a later run)", d.kind(), d.name, why, d.kind(), d.name, d.kind())) + } // replaceLoopAdvice is what `create or modify` and `alter` both say about a diff --git a/mdl/executor/flow_verdict_test.go b/mdl/executor/flow_verdict_test.go index 5020e1c75e..a86bc3b62e 100644 --- a/mdl/executor/flow_verdict_test.go +++ b/mdl/executor/flow_verdict_test.go @@ -31,6 +31,12 @@ func TestFlowRefusalNamesTheFlowAndTheReason(t *testing.T) { if !strings.Contains(msg, "Change activities with `alter microflow M.F") { t.Errorf("control: a change outside a loop keeps the generic alter advice: %q", msg) } + // A drop in one run and a create in the next loses the grants (#944): + // the advice to rebuild says to do both in one script, and why. + if !strings.Contains(msg, "drop the microflow and create it in the same script") || + !strings.Contains(msg, "not to one in a later run") { + t.Errorf("the rebuild advice must keep drop and create in one script: %q", msg) + } // Inside a loop alter cannot splice either: the refusal names the alter // that replaces the loop, closing a while with `end while;`. From dad7d975300189a963aaeb95065e40e190cd748f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:52:16 +0000 Subject: [PATCH 03/13] fix(check): MDL-WORKFLOW10 counts a claim made in a called microflow (#943) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A call passing the task to a callee that claims it — created in the script, or stored (exec -p / check -p via StoredTaskClaimViolations) — counts as a claim; nested calls recurse with a depth limit. An unresolvable callee is a possible claim. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/exec_preflight.go | 3 + mdl/executor/validate_task_claim.go | 166 ++++++++++++++++++++- mdl/executor/validate_task_claim_stored.go | 83 +++++++++++ mdl/executor/validate_task_claim_test.go | 105 +++++++++++++ 4 files changed, 350 insertions(+), 7 deletions(-) create mode 100644 mdl/executor/validate_task_claim_stored.go diff --git a/cmd/mxcli/exec_preflight.go b/cmd/mxcli/exec_preflight.go index 5a6af9d74c..1649531240 100644 --- a/cmd/mxcli/exec_preflight.go +++ b/cmd/mxcli/exec_preflight.go @@ -37,6 +37,9 @@ func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath, scri if exec != nil { if b := exec.Backend(); b != nil { violations = executor.DropSettledCommitNotes(violations, prog, executor.NewStoredCommitEvents(b)) + // A called microflow the script does not create is read from the + // project for MDL-WORKFLOW10 (ako/mxcli#943). + violations = append(violations, executor.StoredTaskClaimViolations(prog, b)...) } } printPreflightViolations(violations, script, showInfo, w, color) diff --git a/mdl/executor/validate_task_claim.go b/mdl/executor/validate_task_claim.go index a0b191d01e..dba25ab3eb 100644 --- a/mdl/executor/validate_task_claim.go +++ b/mdl/executor/validate_task_claim.go @@ -20,8 +20,10 @@ // set task outcome $Task 'Plan'; // // A WARNING, not an error, and deliberately so: a task can legitimately be -// claimed somewhere this rule cannot see — in a microflow this one calls, in a -// nanoflow on the button, or by an earlier step in the process. Reporting those +// claimed somewhere this rule cannot see — in a nanoflow on the button, or by an +// earlier step in the process. A microflow this one calls IS read, when the +// script creates it or (with a project) the project holds it; one that cannot +// be found counts as a possible claim (ako/mxcli#943). Reporting those // as errors would block correct apps. What the rule can say for certain is that // THIS microflow completes a task it never assigned, which is the shape that // fails. @@ -32,6 +34,7 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/linter" ) @@ -41,29 +44,166 @@ import ( const assigneesAssociation = "workflowusertask_assignees" // ValidateTaskClaims reports SET TASK OUTCOME statements whose task was not -// assigned earlier in the same microflow. +// assigned earlier in the same microflow — or in a microflow it calls first, +// passing the task (ako/mxcli#943). A callee the script itself creates is read; +// one it does not create cannot be, so a call that passes the task to it counts +// as a possible claim and the outcome is not reported. With a project, +// StoredTaskClaimViolations reads those callees too. func ValidateTaskClaims(prog *ast.Program) []linter.Violation { + return validateTaskClaims(prog, nil) +} + +// storedClaimSource reads a microflow the script does not create: whether it +// claims its parameter param directly, and which calls it passes param on to +// (callee, callee parameter). found is false when the project has no such +// microflow, which counts as a possible claim. +type storedClaimSource interface { + TaskParameterClaims(flow, param string) (claims bool, calls [][2]string, found bool) +} + +// maxClaimCallDepth bounds the walk through nested calls. Deeper than this the +// call is treated as a possible claim — the quiet direction. +const maxClaimCallDepth = 8 + +func validateTaskClaims(prog *ast.Program, stored storedClaimSource) []linter.Violation { if prog == nil { return nil } + r := &taskClaimResolver{script: map[string]*ast.CreateMicroflowStmt{}, stored: stored} + for _, stmt := range prog.Statements { + if mf, ok := stmt.(*ast.CreateMicroflowStmt); ok { + r.script[strings.ToLower(mf.Name.String())] = mf + } + } var out []linter.Violation for _, stmt := range prog.Statements { mf, ok := stmt.(*ast.CreateMicroflowStmt) if !ok { continue } - out = append(out, taskClaimViolations(mf.Name, mf.Body)...) + out = append(out, r.taskClaimViolations(mf.Name, mf.Body)...) + } + return out +} + +// StoredTaskClaimViolations is the MDL-WORKFLOW10 warnings ValidateTaskClaims +// stays quiet about because the claiming callee is not in the script: with the +// project's microflows readable, a call to a stored microflow that does not +// claim the task it is passed no longer hides the outcome. Only the warnings +// ValidateTaskClaims did not already report are returned, so the two can be +// printed together. +func StoredTaskClaimViolations(prog *ast.Program, b backend.FullBackend) []linter.Violation { + if prog == nil || b == nil { + return nil + } + base := map[string]bool{} + for _, v := range validateTaskClaims(prog, nil) { + base[v.Location.DocumentName+"\x00"+v.Message] = true + } + var out []linter.Violation + for _, v := range validateTaskClaims(prog, &storedTaskClaims{b: b}) { + if !base[v.Location.DocumentName+"\x00"+v.Message] { + out = append(out, v) + } } return out } +// taskClaimResolver knows the script's microflows and, optionally, the +// project's, and answers whether a call claims the task it passes. +type taskClaimResolver struct { + script map[string]*ast.CreateMicroflowStmt + stored storedClaimSource +} + +// callClaims reports whether calling flow with the task bound to param may +// claim it. An unresolvable callee may, so it returns true. +func (r *taskClaimResolver) callClaims(flow, param string, depth int, visited map[string]bool) bool { + key := strings.ToLower(flow) + "\x00" + strings.ToLower(param) + if depth > maxClaimCallDepth { + return true + } + if visited[key] { + // Recursion: the cycle itself claims nothing; another path may. + return false + } + visited[key] = true + if mf, ok := r.script[strings.ToLower(flow)]; ok { + return r.bodyClaims(mf.Body, param, depth+1, visited) + } + if r.stored != nil { + if claims, calls, found := r.stored.TaskParameterClaims(flow, param); found { + if claims { + return true + } + for _, c := range calls { + if r.callClaims(c[0], c[1], depth+1, visited) { + return true + } + } + return false + } + } + return true +} + +// bodyClaims reports whether a flow body claims variable v anywhere — order +// does not matter inside a callee, the call as a whole precedes the outcome. +func (r *taskClaimResolver) bodyClaims(body []ast.MicroflowStatement, v string, depth int, visited map[string]bool) bool { + for _, st := range body { + switch s := st.(type) { + case *ast.ChangeObjectStmt: + if strings.EqualFold(s.Variable, v) && changeClaimsTask(s) { + return true + } + case *ast.CallMicroflowStmt: + for _, param := range callParamsBoundTo(s, v) { + if r.callClaims(s.MicroflowName.String(), param, depth, visited) { + return true + } + } + } + if r.bodyClaims(nestedStatements(st), v, depth, visited) { + return true + } + } + return false +} + +// callParamsBoundTo returns the callee parameters a call binds to $v. +func callParamsBoundTo(s *ast.CallMicroflowStmt, v string) []string { + var params []string + for _, a := range s.Arguments { + if name, ok := plainVariable(a.Value); ok && strings.EqualFold(name, v) { + params = append(params, a.Name) + } + } + return params +} + +// plainVariable returns the variable an expression is, when it is nothing else. +func plainVariable(e ast.Expression) (string, bool) { + for { + switch x := e.(type) { + case *ast.SourceExpr: + e = x.Expression + case *ast.ParenExpr: + e = x.Inner + case *ast.VariableExpr: + return strings.TrimPrefix(x.Name, "$"), true + default: + return "", false + } + } +} + // taskClaimViolations walks one flow body in order, remembering which task // variables have been claimed by the time each outcome is set. // // Order matters and is the point: claiming AFTER the outcome does not help, so // the walk records claims as it passes them rather than collecting them all // first. -func taskClaimViolations(name ast.QualifiedName, body []ast.MicroflowStatement) []linter.Violation { +func (r *taskClaimResolver) taskClaimViolations(name ast.QualifiedName, body []ast.MicroflowStatement) []linter.Violation { flowName := name.String() claimed := map[string]bool{} var out []linter.Violation @@ -76,6 +216,17 @@ func taskClaimViolations(name ast.QualifiedName, body []ast.MicroflowStatement) if changeClaimsTask(s) { claimed[strings.ToLower(s.Variable)] = true } + case *ast.CallMicroflowStmt: + // A claim made by a called microflow counts (ako/mxcli#943). + for _, a := range s.Arguments { + v, ok := plainVariable(a.Value) + if !ok || claimed[strings.ToLower(v)] { + continue + } + if r.callClaims(s.MicroflowName.String(), a.Name, 0, map[string]bool{}) { + claimed[strings.ToLower(v)] = true + } + } case *ast.SetTaskOutcomeStmt: v := strings.ToLower(s.WorkflowTaskVariable) if claimed[v] { @@ -100,8 +251,9 @@ func taskClaimViolations(name ast.QualifiedName, body []ast.MicroflowStatement) " change $%s (System.WorkflowUserTask_Assignees = [%%CurrentUser%%]);\n"+ " commit $%s;\n"+ " TARGETING XPATH / TARGETING MICROFLOW decides who may SEE a task; it does not "+ - "assign it. Ignore this if the task is claimed elsewhere — in a microflow this one "+ - "calls, or earlier in the process.", + "assign it. A microflow this one calls first, passing the task, counts when it claims "+ + "it. Ignore this if the task is claimed elsewhere — in a nanoflow on the button, or "+ + "earlier in the process.", s.WorkflowTaskVariable, s.WorkflowTaskVariable), }) } diff --git a/mdl/executor/validate_task_claim_stored.go b/mdl/executor/validate_task_claim_stored.go new file mode 100644 index 0000000000..9accbd452b --- /dev/null +++ b/mdl/executor/validate_task_claim_stored.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// storedTaskClaims reads stored microflows for MDL-WORKFLOW10: does a callee +// the script does not create claim the task it is passed (ako/mxcli#943)? +type storedTaskClaims struct { + b backend.FullBackend +} + +// TaskParameterClaims reports whether the stored microflow flow writes +// System.WorkflowUserTask_Assignees on its parameter param, and which calls it +// passes param on to. The stored flow is a graph, so every activity counts +// wherever it sits — a claim on any path is accepted, as for a script flow. +func (s *storedTaskClaims) TaskParameterClaims(flow, param string) (bool, [][2]string, bool) { + objs, found := (&StoredCommitEvents{b: s.b}).flowObjects(false, flow) + if !found { + return false, nil, false + } + claims := false + var calls [][2]string + var walk func(*microflows.MicroflowObjectCollection) + walk = func(oc *microflows.MicroflowObjectCollection) { + if oc == nil { + return + } + for _, o := range oc.Objects { + switch a := o.(type) { + case *microflows.ActionActivity: + switch act := a.Action.(type) { + case *microflows.ChangeObjectAction: + if strings.EqualFold(act.ChangeVariable, param) && storedChangeClaims(act) { + claims = true + } + case *microflows.MicroflowCallAction: + if act.MicroflowCall == nil { + continue + } + for _, m := range act.MicroflowCall.ParameterMappings { + if m == nil || !strings.EqualFold(strings.TrimSpace(m.Argument), "$"+param) { + continue + } + p := m.Parameter + if i := strings.LastIndex(p, "."); i >= 0 { + p = p[i+1:] + } + calls = append(calls, [2]string{act.MicroflowCall.Microflow, p}) + } + } + case *microflows.LoopedActivity: + walk(a.ObjectCollection) + } + } + } + walk(objs) + return claims, calls, true +} + +// storedChangeClaims reports whether a stored Change activity writes the +// Assignees association. +func storedChangeClaims(act *microflows.ChangeObjectAction) bool { + for _, c := range act.Changes { + if c == nil { + continue + } + for _, name := range []string{c.AssociationQualifiedName, c.AttributeQualifiedName} { + if i := strings.LastIndex(name, "."); i >= 0 { + name = name[i+1:] + } + if strings.EqualFold(name, assigneesAssociation) { + return true + } + } + } + return false +} diff --git a/mdl/executor/validate_task_claim_test.go b/mdl/executor/validate_task_claim_test.go index aecb3c4e25..29b076d167 100644 --- a/mdl/executor/validate_task_claim_test.go +++ b/mdl/executor/validate_task_claim_test.go @@ -95,3 +95,108 @@ func TestAssigneesSpellingsAreAllRecognised(t *testing.T) { } } } + +// ako/mxcli#943: the claim made in a called sub-microflow. + +func taskClaimWarningsIn(t *testing.T, src string, stored storedClaimSource) []string { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var msgs []string + for _, v := range validateTaskClaims(prog, stored) { + msgs = append(msgs, v.Message) + } + return msgs +} + +const claimingSub = `create microflow M.SUB_Claim ( $T: System.WorkflowUserTask ) +begin + change $T (System.WorkflowUserTask_Assignees = [%CurrentUser%]); + commit $T; +end; +` + +const nonClaimingSub = `create microflow M.SUB_Claim ( $T: System.WorkflowUserTask ) +begin + log info 'nothing'; +end; +` + +const callerOfSub = `create microflow M.ACT ( $Task: System.WorkflowUserTask ) +begin + call microflow M.SUB_Claim(T = $Task); + set task outcome $Task 'Plan'; +end; +` + +func TestClaimInScriptCalleeCounts(t *testing.T) { + if got := taskClaimWarningsIn(t, claimingSub+callerOfSub, nil); len(got) != 0 { + t.Errorf("a claim in a called microflow was not counted: %v", got) + } +} + +func TestScriptCalleeThatDoesNotClaimStillWarns(t *testing.T) { + // The control: the callee is read, not just assumed to claim. + if got := taskClaimWarningsIn(t, nonClaimingSub+callerOfSub, nil); len(got) != 1 { + t.Errorf("got %d warnings, want 1 for a callee that does not claim: %v", len(got), got) + } +} + +func TestCalleeClaimingAnotherParameterStillWarns(t *testing.T) { + src := `create microflow M.SUB_Claim ( $T: System.WorkflowUserTask, $Other: System.WorkflowUserTask ) +begin + change $Other (System.WorkflowUserTask_Assignees = [%CurrentUser%]); +end; +` + callerOfSub + if got := taskClaimWarningsIn(t, src, nil); len(got) != 1 { + t.Errorf("a claim on a different callee parameter counted: %v", got) + } +} + +func TestUnresolvableCalleeIsAPossibleClaim(t *testing.T) { + if got := taskClaimWarningsIn(t, callerOfSub, nil); len(got) != 0 { + t.Errorf("a call to an unknown microflow passing the task was warned: %v", got) + } +} + +func TestNestedScriptCalleesAndRecursion(t *testing.T) { + src := `create microflow M.SUB_Inner ( $X: System.WorkflowUserTask ) +begin + change $X (System.WorkflowUserTask_Assignees = [%CurrentUser%]); +end; +create microflow M.SUB_Claim ( $T: System.WorkflowUserTask ) +begin + call microflow M.SUB_Claim(T = $T); + call microflow M.SUB_Inner(X = $T); +end; +` + callerOfSub + if got := taskClaimWarningsIn(t, src, nil); len(got) != 0 { + t.Errorf("a claim two calls deep was not counted: %v", got) + } + loop := `create microflow M.SUB_Claim ( $T: System.WorkflowUserTask ) +begin + call microflow M.SUB_Claim(T = $T); +end; +` + callerOfSub + if got := taskClaimWarningsIn(t, loop, nil); len(got) != 1 { + t.Errorf("a self-recursive callee that never claims: got %v, want 1 warning", got) + } +} + +type fakeStoredClaims map[string]bool // "flow.param" -> claims + +func (f fakeStoredClaims) TaskParameterClaims(flow, param string) (bool, [][2]string, bool) { + c, ok := f[flow+"."+param] + return c, nil, ok +} + +func TestStoredCalleeIsRead(t *testing.T) { + if got := taskClaimWarningsIn(t, callerOfSub, fakeStoredClaims{"M.SUB_Claim.T": true}); len(got) != 0 { + t.Errorf("a stored callee that claims was warned: %v", got) + } + if got := taskClaimWarningsIn(t, callerOfSub, fakeStoredClaims{"M.SUB_Claim.T": false}); len(got) != 1 { + t.Errorf("a stored callee that does not claim: got %v, want 1 warning", got) + } +} From 0bd2766f7e2490b3c05a94f26fc6a3a6fa1f9473 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:52:16 +0000 Subject: [PATCH 04/13] fix(check): check -p drops a settled MDL067 and reads stored callees (#943) check connects to the project before the semantic report and applies the DropSettledCommitNotes filter exec's preflight applies. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/check_stored_semantics_test.go | 100 +++++++++++++++++++++++ cmd/mxcli/cmd_check.go | 37 ++++++--- 2 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 cmd/mxcli/check_stored_semantics_test.go diff --git a/cmd/mxcli/check_stored_semantics_test.go b/cmd/mxcli/check_stored_semantics_test.go new file mode 100644 index 0000000000..14002e3960 --- /dev/null +++ b/cmd/mxcli/check_stored_semantics_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// ako/mxcli#943: two semantic rules answer differently once the stored model +// is read, and `check -p` did not read it where `exec -p` did. +// +// - MDL067 (a bare commit now means WITH events) is noise for a flow already +// stored that way; exec dropped it, check printed it. Control: a flow +// stored WITHOUT events, which the bare commit flips, is still noted. +// - MDL-WORKFLOW10 ignored a claim made in a called microflow. A stored +// callee that claims silences it; control: one that does not still warns. +func TestCheckProject_ReadsStoredFlowsForSemanticRules(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyTree(src, dir); err != nil { + t.Fatal(err) + } + mpr := filepath.Join(dir, "PedApp.mpr") + + const setup = `create or modify microflow MyFirstModule.Settled ($U: System.User) +begin + commit $U; +end; +create or modify microflow MyFirstModule.Flipped ($U: System.User) +begin + commit $U without events; +end; +create or modify microflow MyFirstModule.SUB_Claim ($T: System.WorkflowUserTask) +begin + change $T (System.WorkflowUserTask_Assignees = [%CurrentUser%]); +end; +create or modify microflow MyFirstModule.SUB_NoClaim ($T: System.WorkflowUserTask) +begin + log info 'nothing'; +end; +` + exe := executor.New(io.Discard) + exe.SetBackendFactory(newBackendFactory()) + prog, errs := visitor.Build(fmt.Sprintf("connect local '%s';\n%s", visitor.QuoteString(mpr), setup)) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + if err := exe.ExecuteProgram(prog); err != nil { + t.Fatalf("store the flows: %v", err) + } + _ = exe.Close() + + _ = checkCmd.InheritedFlags() + _ = rootCmd.PersistentFlags().Set("project", mpr) + defer func() { + _ = rootCmd.PersistentFlags().Set("project", "") + rootCmd.PersistentFlags().Lookup("project").Changed = false + }() + + check := func(script string) string { + file := writeScript(t, t.TempDir(), "s.mdl", "mdl 1;\n"+script) + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{file}) }) + if code != 0 { + t.Fatalf("check failed (exit %d):\n%s", code, out) + } + return out + } + + settled := check("create or modify microflow MyFirstModule.Settled ($U: System.User)\nbegin\n commit $U;\nend;\n") + if strings.Contains(settled, "MDL067") { + t.Errorf("MDL067 printed for a commit stored exactly as the script writes it:\n%s", settled) + } + flipped := check("create or modify microflow MyFirstModule.Flipped ($U: System.User)\nbegin\n commit $U;\nend;\n") + if !strings.Contains(flipped, "MDL067") { + t.Errorf("control: MDL067 missing for a bare commit that flips the stored events:\n%s", flipped) + } + + claimed := check("create or modify microflow MyFirstModule.ACT_A ($Task: System.WorkflowUserTask)\nbegin\n" + + " call microflow MyFirstModule.SUB_Claim(T = $Task);\n set task outcome $Task 'Plan';\nend;\n") + if strings.Contains(claimed, "MDL-WORKFLOW10") { + t.Errorf("MDL-WORKFLOW10 for a task claimed by the stored callee:\n%s", claimed) + } + unclaimed := check("create or modify microflow MyFirstModule.ACT_B ($Task: System.WorkflowUserTask)\nbegin\n" + + " call microflow MyFirstModule.SUB_NoClaim(T = $Task);\n set task outcome $Task 'Plan';\nend;\n") + if !strings.Contains(unclaimed, "MDL-WORKFLOW10") { + t.Errorf("control: no MDL-WORKFLOW10 for a stored callee that does not claim:\n%s", unclaimed) + } +} diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 504d186656..d7d880b4b2 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -262,6 +262,30 @@ func runCheckFile(cmd *cobra.Command, filePath string) int { // refuses exactly what `mxcli check` reports. Adding a check there gives // both commands it at once. violations := append(testProblems, executor.ValidateProgram(prog, projectPath)...) + + // With a project, connect before reporting: two semantic rules read the + // stored model, so `check -p` reports what `exec -p` would (ako/mxcli#943). + // - MDL067 is dropped for a flow already stored the way the script + // commits — the filter exec's preflight applies. + // - MDL-WORKFLOW10 reads a called microflow the script does not create. + var exec *executor.Executor + if projectPath != "" { + e, logger := newLoggedExecutorTo("check", progressSink(format)) + defer logger.Close() + defer e.Close() + connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath))) + for _, stmt := range connectProg.Statements { + if err := e.Execute(stmt); err != nil { + fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err) + return 1 + } + } + exec = e + if b := exec.Backend(); b != nil { + violations = executor.DropSettledCommitNotes(violations, prog, executor.NewStoredCommitEvents(b)) + violations = append(violations, executor.StoredTaskClaimViolations(prog, b)...) + } + } violations = executor.ApplyDeprecationPolicy(violations, depPolicy) if isStructured { @@ -290,18 +314,7 @@ func runCheckFile(cmd *cobra.Command, filePath string) int { fmt.Printf("\nValidating references against: %s\n", projectPath) fmt.Printf("(Note: References to objects created within the script are skipped)\n") } - exec, logger := newLoggedExecutorTo("check", progressSink(format)) - defer logger.Close() - defer exec.Close() - - // Connect to project - connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath))) - for _, stmt := range connectProg.Statements { - if err := exec.Execute(stmt); err != nil { - fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err) - return 1 - } - } + // exec was connected above, before the semantic report. // A test file's microflows live in the runner's MxTest module, which // the runner creates first and the project does not have From df686357f438e2033eb384a5d9b04c90c9bbe95e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:52:25 +0000 Subject: [PATCH 05/13] fix(test): the endpoint-registration script is mdl 1 (#943) mxcli test no longer warns MDL-DEPR001 / MDL-V1-SLASH about its own script. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/testrunner/endpoint.go | 18 ++++++++---- cmd/mxcli/testrunner/endpoint_clean_test.go | 31 +++++++++++++++++++++ cmd/mxcli/testrunner/endpoint_test.go | 4 +-- 3 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 cmd/mxcli/testrunner/endpoint_clean_test.go diff --git a/cmd/mxcli/testrunner/endpoint.go b/cmd/mxcli/testrunner/endpoint.go index 3f5cb4e30a..3b2061aa68 100644 --- a/cmd/mxcli/testrunner/endpoint.go +++ b/cmd/mxcli/testrunner/endpoint.go @@ -7,6 +7,8 @@ import ( "encoding/hex" "fmt" "strings" + + "github.com/mendixlabs/mxcli/mdl/langver" ) const ( @@ -74,15 +76,22 @@ func testFlowName(tc TestCase) string { return testFlowPrefix + tc.ID } func GenerateEndpointMDL(chainAfterStartup string) string { var b strings.Builder + // The canonical spelling under the current language version: `mdl 1;`, + // `create or modify` and `;` alone as the terminator. The runner execs this + // script itself, and the deprecated forms made every `mxcli test` print + // MDL-DEPR001 and MDL-V1-SLASH warnings about a script the user never wrote + // (ako/mxcli#943). + writeScriptHeader(&b, langver.V1) + b.WriteString("\n") b.WriteString("CREATE MODULE " + mxTestModule + ";\n\n") b.WriteString("/** Registers the mxcli test endpoint. Called once at startup. */\n") - b.WriteString("CREATE OR REPLACE JAVA ACTION " + endpointRegisterAction + "() RETURNS Boolean\n") + b.WriteString("CREATE OR MODIFY JAVA ACTION " + endpointRegisterAction + "() RETURNS Boolean\n") b.WriteString("AS $$\n") b.WriteString(endpointJava) - b.WriteString("\n$$;\n/\n\n") + b.WriteString("\n$$;\n\n") b.WriteString("/** Registers the mxcli test endpoint at boot. Runs no tests. */\n") - b.WriteString("CREATE OR REPLACE MICROFLOW " + endpointStartupFlow + " ()\n") + b.WriteString(createFlow(langver.V1) + " " + endpointStartupFlow + " ()\n") b.WriteString("RETURNS Boolean AS $Registered\n") b.WriteString("BEGIN\n") b.WriteString(" $Registered = CALL JAVA ACTION " + endpointRegisterAction + "();\n") @@ -93,8 +102,7 @@ func GenerateEndpointMDL(chainAfterStartup string) string { b.WriteString(" $Chained = CALL MICROFLOW " + chainAfterStartup + "();\n") } b.WriteString(" RETURN $Registered;\n") - b.WriteString("END;\n") - b.WriteString("/\n") + writeFlowEnd(&b, langver.V1) return b.String() } diff --git a/cmd/mxcli/testrunner/endpoint_clean_test.go b/cmd/mxcli/testrunner/endpoint_clean_test.go new file mode 100644 index 0000000000..879daae306 --- /dev/null +++ b/cmd/mxcli/testrunner/endpoint_clean_test.go @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/langver" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestGenerateEndpointMDLDrawsNoDiagnostics: the runner execs this script +// itself, so anything exec's preflight says about it is noise the user cannot +// act on. It printed two MDL-DEPR001 (`create or replace`) and two +// MDL-V1-SLASH per `mxcli test` run (ako/mxcli#943). +func TestGenerateEndpointMDLDrawsNoDiagnostics(t *testing.T) { + for _, chain := range []string{"", "MyModule.ASU_Startup"} { + src := GenerateEndpointMDL(chain) + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("chain %q: parse errors: %v\n%s", chain, errs, src) + } + if prog.LanguageVersion != langver.V1 { + t.Errorf("chain %q: script is read as language version %v, want mdl 1", chain, prog.LanguageVersion) + } + for _, v := range executor.ValidateProgram(prog, "") { + t.Errorf("chain %q: %s %s: %s", chain, v.Severity, v.RuleID, v.Message) + } + } +} diff --git a/cmd/mxcli/testrunner/endpoint_test.go b/cmd/mxcli/testrunner/endpoint_test.go index 7e8f3ff8e4..63e71d8408 100644 --- a/cmd/mxcli/testrunner/endpoint_test.go +++ b/cmd/mxcli/testrunner/endpoint_test.go @@ -137,8 +137,8 @@ func TestGenerateEndpointMDLShape(t *testing.T) { mdl := GenerateEndpointMDL("") for _, want := range []string{ "CREATE MODULE " + mxTestModule + ";", - "CREATE OR REPLACE JAVA ACTION " + endpointRegisterAction + "() RETURNS Boolean", - "CREATE OR REPLACE MICROFLOW " + endpointStartupFlow + " ()", + "CREATE OR MODIFY JAVA ACTION " + endpointRegisterAction + "() RETURNS Boolean", + "CREATE OR MODIFY MICROFLOW " + endpointStartupFlow + " ()", "RETURNS Boolean AS $Registered", } { if !strings.Contains(mdl, want) { From 5eaa99672d53397607c5acf7175ba8c227719a14 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:52:25 +0000 Subject: [PATCH 06/13] fix(check): MDL-WIDGET15 skips styled texts; widget rules carry a location (#943) Co-Authored-By: Claude Opus 5.5 --- mdl/executor/validate_widgets.go | 59 +++++++++++++++++++++++---- mdl/executor/validate_widgets_test.go | 41 +++++++++++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 867ea8d6b3..7d70d68813 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -111,6 +111,38 @@ func ValidateWidgetPropertiesForStatement(stmt ast.Statement, registry *WidgetRe if registry == nil { return nil } + return withDocumentLocation(validateWidgetPropertiesOf(stmt, registry), stmt) +} + +// withDocumentLocation fills in the document a widget violation belongs to. +// The widget rules name it only in the message, so a report grouped by +// location printed them under "(no module)" with a blank "at" (ako/mxcli#943). +// A violation that already carries a location keeps it. +func withDocumentLocation(vs []linter.Violation, stmt ast.Statement) []linter.Violation { + var loc linter.Location + switch s := stmt.(type) { + case *ast.CreatePageStmtV3: + loc = linter.Location{Module: s.Name.Module, DocumentType: "page", DocumentName: s.Name.Name} + case *ast.CreateSnippetStmtV3: + loc = linter.Location{Module: s.Name.Module, DocumentType: "snippet", DocumentName: s.Name.Name} + case *ast.AlterPageStmt: + kind := "page" + if strings.EqualFold(s.ContainerType, "snippet") { + kind = "snippet" + } + loc = linter.Location{Module: s.PageName.Module, DocumentType: kind, DocumentName: s.PageName.Name} + default: + return vs + } + for i := range vs { + if vs[i].Location == (linter.Location{}) { + vs[i].Location = loc + } + } + return vs +} + +func validateWidgetPropertiesOf(stmt ast.Statement, registry *WidgetRegistry) []linter.Violation { if label, widgets, ok := documentWidgets(stmt); ok { out := validateWidgetTree(widgets, registry, label) out = append(out, validateNamedObjectBindings(widgets, documentEntityParameters(stmt), label)...) @@ -359,29 +391,42 @@ func inlineDynamicText(w *ast.WidgetV3) bool { return !headingRenderModeRe.MatchString(w.GetRenderMode()) } +// styledDynamicText reports whether a dynamictext carries its own class or +// style. The author has laid it out — a theme class that makes it a block, a +// margin, a flex item — so the "no separator" advice does not apply to it. +func styledDynamicText(w *ast.WidgetV3) bool { + return strings.TrimSpace(w.GetClass()) != "" || strings.TrimSpace(w.GetStyle()) != "" || + strings.TrimSpace(w.GetDynamicClasses()) != "" +} + // validateConsecutiveDynamicText emits an advisory (MDL-WIDGET15) when two or // more INLINE dynamictext widgets are direct siblings: Mendix renders a Text- or // Paragraph-mode DynamicText inline (a ``), so adjacent ones concatenate // with no separator (`€ 310` + `7/24/2026` → `€ 3107/24/2026`). Only a heading // render mode (H1–H6) is block-level and breaks the run. Info severity — it does // not fail the build, it warns the author about a layout surprise. (ledger #27/#29) +// +// A text with its own `class:` or `style:` is not counted and breaks the run: +// its layout is the author's, not Atlas's default inline span. Report pages +// that lay out label/value pairs with SCSS drew ~35 of these notes, none of +// them about text that actually fused (ako/mxcli#943). func validateConsecutiveDynamicText(siblings []*ast.WidgetV3, locationPrefix string) []linter.Violation { - run := 0 + var run []*ast.WidgetV3 for _, w := range siblings { - if inlineDynamicText(w) { - run++ + if inlineDynamicText(w) && !styledDynamicText(w) { + run = append(run, w) } else { - run = 0 + run = nil } // Emit once, on the second inline dynamictext of a run, so a group of N // only warns once. - if run == 2 { + if len(run) == 2 { return []linter.Violation{{ RuleID: "MDL-WIDGET15", Severity: linter.SeverityInfo, Message: fmt.Sprintf( - "%s: adjacent inline dynamictext widgets (RenderMode Text or Paragraph, both ) render with no separator, so their text concatenates. Merge them into one dynamictext with multiple content params, wrap each in its own container, or use a heading RenderMode (H1–H6, which is block-level). Note: Paragraph does NOT fix this — it also renders inline.", - locationPrefix), + "%s: adjacent inline dynamictext widgets — %s and %s — (RenderMode Text or Paragraph, both ) render with no separator, so their text concatenates. Merge them into one dynamictext with multiple content params, wrap each in its own container, give them a class or style that lays them out, or use a heading RenderMode (H1–H6, which is block-level). Note: Paragraph does NOT fix this — it also renders inline.", + locationPrefix, widgetLabel(run[0].Name, run[0].Type), widgetLabel(run[1].Name, run[1].Type)), }} } } diff --git a/mdl/executor/validate_widgets_test.go b/mdl/executor/validate_widgets_test.go index e3e1583022..13bf642cb0 100644 --- a/mdl/executor/validate_widgets_test.go +++ b/mdl/executor/validate_widgets_test.go @@ -9,6 +9,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" ) // Issue #650 — MDL-WIDGET04 flags a dynamictext whose template references a {N} @@ -300,6 +301,9 @@ func TestValidateConsecutiveDynamicText(t *testing.T) { dtRM := func(name, rm string) *ast.WidgetV3 { return &ast.WidgetV3{Type: "dynamictext", Name: name, Properties: map[string]any{"RenderMode": rm}} } + dtProp := func(name, key, val string) *ast.WidgetV3 { + return &ast.WidgetV3{Type: "dynamictext", Name: name, Properties: map[string]any{key: val}} + } tb := func(name string) *ast.WidgetV3 { return &ast.WidgetV3{Type: "textbox", Name: name} } cases := []struct { name string @@ -320,6 +324,12 @@ func TestValidateConsecutiveDynamicText(t *testing.T) { {"heading then subtitle", []*ast.WidgetV3{dtRM("h", "H2"), dt("sub")}, false}, {"two headings", []*ast.WidgetV3{dtRM("h1", "H2"), dtRM("h2", "H3")}, false}, {"heading breaks a run of inlines", []*ast.WidgetV3{dt("a"), dtRM("h", "H2"), dt("b")}, false}, + // ako/mxcli#943: a text with its own class or style is laid out by the + // author's SCSS, and BankV1's report pages drew ~35 notes for them. + {"each with its own class", []*ast.WidgetV3{dtProp("a", "Class", "lbl"), dtProp("b", "Class", "val")}, false}, + {"each with its own style", []*ast.WidgetV3{dtProp("a", "Style", "display:block"), dtProp("b", "Style", "display:block")}, false}, + {"a styled text breaks the run", []*ast.WidgetV3{dt("a"), dtProp("b", "Class", "val"), dt("c")}, false}, + {"two classless after a styled one", []*ast.WidgetV3{dtProp("a", "Class", "lbl"), dt("b"), dt("c")}, true}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -331,6 +341,37 @@ func TestValidateConsecutiveDynamicText(t *testing.T) { } } +// TestConsecutiveDynamicTextHasALocation: MDL-WIDGET15 printed under +// "(no module)" with a blank "at", so a report of 35 could not be traced to a +// page without reading every message (ako/mxcli#943). +func TestConsecutiveDynamicTextHasALocation(t *testing.T) { + prog, errs := visitor.Build(`create page Shop.Report (title: 'R', layout: Atlas_Core.Atlas_Default) { + dynamictext txtA (content: 'a') + dynamictext txtB (content: 'b') +}`) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + registry := LoadWidgetRegistry("") + var found bool + for _, v := range ValidateWidgetPropertiesForStatement(prog.Statements[0], registry) { + if v.RuleID != "MDL-WIDGET15" { + continue + } + found = true + want := linter.Location{Module: "Shop", DocumentType: "page", DocumentName: "Report"} + if v.Location != want { + t.Errorf("location = %+v, want %+v", v.Location, want) + } + if !strings.Contains(v.Message, "txtA") || !strings.Contains(v.Message, "txtB") { + t.Errorf("the message does not name the widgets: %s", v.Message) + } + } + if !found { + t.Fatal("no MDL-WIDGET15 for two adjacent classless texts") + } +} + // TestValidateObjectListItemEnums — MDL-WIDGET08 flags an object-list item's // enumeration sub-property whose value isn't a declared member key (e.g. a Maps // marker LocationType outside {address, latlng}). Studio Pro silently defaults From 51be27ee89304b99baa1454e125a3210baaa9c87 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 11:52:25 +0000 Subject: [PATCH 07/13] docs: findings and changelog for #943 Co-Authored-By: Claude Opus 5.5 --- .claude/skills/fix-issue/findings/cmd-mxcli.jsonl | 2 ++ .claude/skills/fix-issue/findings/mdl-executor.jsonl | 2 ++ CHANGELOG.md | 1 + 3 files changed, 5 insertions(+) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index a5cc6021f8..0d9ae0a6de 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -149,3 +149,5 @@ {"date": "2026-10-02", "area": "cmd/mxcli", "symptom": "`run --local --watch` on Mendix 11.13: a page added while the loop runs is never bundled — the apply reports success (often 'applied via reload'), but opening the page 404s on dist/pages/..js and the page stays blank; restarting the loop fixes it", "cause": "mxbuild's tools/node/rollup-plugin-mendix-pages.mjs (byte-identical in 11.13.0 and 11.14.0) globs web/pages only at bundler start: watchChange compares path.relative(cwd, id) against PAGES_FOLDER=\"./pages\" + \"/\", a './' prefix relative() never yields, so shouldRefreshPageFiles never flips. mxcli could not see it: ensureClientServed probes index.js and its static imports, and pages are dynamic imports", "fix": "missingPageChunks compares web/pages/**/*.js against web/dist/pages/**/*.js (shape-gated: needs web/pages and web/dist/index.js, so 11.14 prebuilt and classic are no-ops); under --watch watchAndApply restarts the WebClientWatcher (a fresh rollup run re-globs) and reports one line; ensureClientServed does a one-shot BuildWebClient for the same condition. RunLocal stops whichever watcher is current at exit", "insight": "A stand-alone rollup watch repro (pages/A.js, then add pages/B.js) isolates it in seconds: the 'change pages/B.js create' event fires but the next bundle still emits only A; a fresh watcher, or the plugin with PAGES_FOLDER=\"pages\", emits both. Restart rather than one-shot: the stale watcher would also never rebuild the new page when it is edited later. A structural change (navigation) clears web/dist and the old index.js fallback re-bundles everything, masking the bug; a page-only add applied via reload is the reproducing case. E2E 11.13: pre-change binary 404 on the new chunk; fixed binary 200 + page text, and a follow-up edit of the same page re-bundled incrementally with no restart (control)", "file": "cmd/mxcli/docker/webclient_pages.go (missingPageChunks, recoverMissingPages); cmd/mxcli/docker/runlocal.go (watchAndApply, ensureClientServed, RunLocal watcher defer)", "test": "cmd/mxcli/docker/webclient_pages_test.go"} {"date": "2026-10-02", "area": "cmd/mxcli", "symptom": "A `.test.mdl` starting with `mdl 1;` loses its first test: `mxcli test --list` finds 41 of 42 with no message; `mxcli check` on it reads the bodies as mdl 0 (MDL-V1-SLASH / MDL-V1-LIMIT1 warnings fire under a file that says mdl 1); the generated runner scripts end every flow with `/`, which mdl 1 refuses; and `fmt --upgrade --header` adds no header to a test file", "cause": "Nothing in the test format read the header. It was body text in the first `/`-chunk, so that chunk's `/** @test */` was no longer a LEADING doc comment (scanDocComments) and parseMDLTests skipped it silently; CheckSource rendered only the bodies (dropping the header line) and put `END; /` on separators; GenerateTestRunner/GenerateTestFlows always wrote `/` and `create or replace`", "fix": "takeLanguageHeader reads the header with langver.HeaderSpan (the grammar's own rule: first token after trivia) and blanks it in place so lines/columns hold; TestCase carries Version + HeaderLine (markdown: per mdl-test block); CheckSource renders the header on its line and closes wrappers with `END;` (no `/`, any version); the generators write the header, `create or modify` and no `/` under mdl 1; parseTestFiles refuses a suite mixing versions (one suite = one script = one header); UpgradeSource honours AddHeader and writes the header at file top / inside each mdl-test block; UpgradeSource maps the upgraded rendering back with a line diff (difflib opcodes, every changed region must be body lines) instead of an index walk, because the MDL-V1-ESCAPE rewrite of `\\n` writes a real line break and adds a line; writeBodyLines no longer indents a body line that starts inside a string literal", "insight": "A header is not just a flag to pass along — in a format that is NOT parsed as one script, every consumer that splits the text has to take it out first, or it becomes content in whichever chunk it lands in. The silent drop came from the same 'leading doc comment' rule as #927; a per-test Version also forced the question 'what is a suite's version', which only refusal answers safely. The runtime run was what found the last defect: unit tests and `mx check` were green while an upgraded test asserting length 3 saw 5 — the generators indented the continuation line of a multi-line string literal, so the indent became part of the value. Compare an mdl 0 file, its fmt --upgrade output, and a headerless control in the running app (both runners)", "issue": "ako/mxcli#847", "file": "cmd/mxcli/testrunner/parser.go (takeLanguageHeader, suiteLanguageVersion); check_source.go; generator.go; generator_endpoint.go; upgrade_source.go; mdl/langver/langver.go (HeaderSpan)", "test": "cmd/mxcli/testrunner/language_header_test.go; e2e: mxcli test --local on a fresh 11.13.0 app, endpoint and --legacy-runner"} {"date": "2026-10-02", "area": "cmd/mxcli/check", "symptom": "`mxcli check x.test.mdl -p app.mpr` reports `module not found: MxTest` once per test and exits 1 on a file `mxcli test` runs green — a test file can never pass check, and a real missing reference in a body is hidden behind it", "cause": "CheckSource wraps each test body in a `MxTest.Check_*` microflow (the runner's module), and the reference pass resolved that module against the project; the runner creates MxTest as the first statement of every generated script and drops it afterwards, so the project never has it", "fix": "testrunner.WithRunnerModule appends `create module if not exists MxTest` to the program used for the reference and project-conflict pass of a test file (appended so `statement N` still numbers the tests; definitions are collected program-wide; `if not exists` keeps a user's own MxTest from reading as a conflict)", "insight": "Seed the module the way the runner does, rather than exempting the file: the control (a body with a real missing entity) must still fail, and before the fix it showed only the MxTest error — the false positive was also masking true ones", "issue": "ako/mxcli#677", "file": "cmd/mxcli/testrunner/check_source.go (WithRunnerModule); cmd/mxcli/cmd_check.go", "test": "cmd/mxcli/check_test_file_refs_test.go"} +{"date": "2026-10-03", "area": "cmd/mxcli/check", "symptom": "`mxcli check -p` prints MDL067 (bare commit now WITH events) for a create-or-modify flow already stored with events, which `exec -p` no longer prints", "cause": "cmd_check ran ValidateProgram without the DropSettledCommitNotes filter exec_preflight applies; the project was only connected later, for the reference tier", "fix": "check connects to the project before the semantic report when -p is given, applies DropSettledCommitNotes and StoredTaskClaimViolations, and reuses that connection for the reference tier", "insight": "Two gates over one rule set drift whenever a post-filter lives in only one of them; grep for every caller of ValidateProgram when adding a filter. Control: a flow stored without events still notes", "issue": "ako/mxcli#943", "file": "cmd/mxcli/cmd_check.go", "test": "cmd/mxcli/check_stored_semantics_test.go"} +{"date": "2026-10-03", "area": "cmd/mxcli/test", "symptom": "every `mxcli test` run prints 2x MDL-DEPR001 and 2x MDL-V1-SLASH about a script the user never wrote", "cause": "GenerateEndpointMDL emitted a headerless mdl 0 script with `create or replace` and `/` terminators; the test-flow generators had already moved to the version-aware writeScriptHeader/createFlow/writeFlowEnd", "fix": "GenerateEndpointMDL writes mdl 1 through the same helpers (header, create or modify, `;` only); endpoint script is independent of the suite's version", "insight": "A generated script is checked like a user's one; pin it with a test that parses it and asserts ValidateProgram returns nothing. Verified end to end with `mxcli test --local` on a fresh 11.13 app", "issue": "ako/mxcli#943", "file": "cmd/mxcli/testrunner/endpoint.go", "test": "cmd/mxcli/testrunner/endpoint_clean_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index c672869457..648716c651 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -836,3 +836,5 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: TestApp WorkflowCommons snippets break exec — `combobox … (Attribute: TimeFrame)` \"has no entity to bind against\", image `Visible: CompletionType in (…)` \"place the widget inside a data container\"; widgets sit directly in a snippet with a parameter, no data view", "cause": "Studio Pro binds a widget outside every data container to a page/snippet PARAMETER: AttributeRef (or ConditionalVisibilitySettings.Attribute, or a combo box IndirectEntityRef) beside SourceVariable Forms$PageVariable {SnippetParameter|PageParameter: name, Widget: \"\"}. describe printed the attribute bare and exec had no spelling for the source, so it refused (or, before describe/pluggable Visible were fixed, silently wrote no binding)", "file": "`mdl/executor/cmd_pages_parameter_binding.go`, `cmd_pages_builder_v3.go` (resolveInputBinding/parameterVariable), `widget_engine.go` (Attribute/Association mappings), `cmd_pages_builder_visible_when.go`, describe in `cmd_pages_describe_parse.go`/`cmd_pages_describe_pluggable.go`, writer `widgetobj.SetSourceVariable`, `conditionalVisibilityToGen`", "insight": "A newly fixed describe gap can surface as an exec refusal the allowlist never expected: the refusal was right for the bare spelling and the cure is a spelling for the stored source (`$Param.Attr`, `$Param.Module.Assoc`, `Visible: $Param.Attr in (…)`), not a looser guard. Survey SourceVariable slot combinations (W/P/S/L) across the fixture first: TestApp had --S- on built-in inputs, pluggable values and 76 visibility settings, all unspelled", "refs": ["#721", "#826"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "describe prints a pluggable image's `Visible: Attr in (…)` (and expression Visible/Editable) twice", "cause": "the image branch appended appendConditionalProps and then appendAppearanceProps, which appends the same conditional settings", "file": "`mdl/executor/cmd_pages_describe_output.go` (image branch)", "insight": "appendAppearanceProps already owns visibility/editability; a branch that also appends them duplicates the key — grep for both on one widget kind", "refs": ["#721"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: describe → exec of TestApp's WorkflowCommons.UserTask_Assign fails: \"widget `grid8` (datagrid) cannot have its visibility set: its widget package declares no Visibility system property\"", "cause": "MDL-WIDGET41 gated Visible: on the package declaring , measured only from which packages declare it — but Studio Pro stores ConditionalVisibilitySettings on a Datagrid whose Type declares no Visibility property, and mxbuild 11.14 accepts static and conditional visibility there (0 errors)", "file": "`mdl/executor/pluggable_system_props.go` (`undeclaredSystemProps`)", "insight": "A declared system property is evidence of where a setting is shown, not of whether it exists; a refusal must be measured against what Studio Pro actually stores. Run the roundtrip harness over Studio Pro-authored pages before adding a refusal on stored shapes. Editability stays gated", "refs": []} +{"date": "2026-10-03", "area": "mdl/executor", "symptom": "MDL-WORKFLOW10 \"completes user task without assigning it first\" fires when the claim is made in a called sub-microflow (`call microflow SUB_Claim(T = $Task); set task outcome $Task ...`)", "cause": "taskClaimViolations only counted a ChangeObjectStmt writing WorkflowUserTask_Assignees in the same body; a CallMicroflowStmt passing the task was invisible", "fix": "taskClaimResolver: a call passing $Task to parameter P counts as a claim when the callee claims P — read from the script's own create microflow, or (check -p / exec -p, StoredTaskClaimViolations) from the stored flow; nested calls recurse with a depth limit and visited set; an unresolvable callee counts as a possible claim", "insight": "A warning that can only see one flow has to fail quiet at the call boundary; resolve what can be resolved and treat the rest as a possible claim. Control: a callee that does not claim (script or stored) still warns", "issue": "ako/mxcli#943", "file": "mdl/executor/validate_task_claim.go, validate_task_claim_stored.go", "test": "mdl/executor/validate_task_claim_test.go; cmd/mxcli/check_stored_semantics_test.go"} +{"date": "2026-10-03", "area": "mdl/executor", "symptom": "MDL-WIDGET15 (adjacent inline dynamictexts concatenate) fires ~35 times on report pages whose texts each carry their own class: laid out by SCSS; and prints under \"(no module)\" with a blank \"at\"", "cause": "validateConsecutiveDynamicText counted every Text/Paragraph dynamictext regardless of class/style; no widget rule set Violation.Location — the document was only in the message prefix", "fix": "a dynamictext with Class, Style or DynamicClasses breaks the run; the message names the two widgets; ValidateWidgetPropertiesForStatement fills Location (module, page|snippet, name) for every widget violation that has none", "insight": "Location belongs at the statement boundary, not in each rule: filling it once in ValidateWidgetPropertiesForStatement fixed every widget rule's blank location at once", "issue": "ako/mxcli#943", "file": "mdl/executor/validate_widgets.go", "test": "mdl/executor/validate_widgets_test.go (TestValidateConsecutiveDynamicText, TestConsecutiveDynamicTextHasALocation)"} diff --git a/CHANGELOG.md b/CHANGELOG.md index f9ac22a378..0b09e2884c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Less noise from `check` and `test`** (ako/mxcli#943) — **MDL-WORKFLOW10** no longer warns when the task is claimed in a called microflow: a callee the script creates is read (nested calls too), and with `-p` a stored one; a call that passes the task to a microflow neither can find counts as a possible claim. A callee that does not claim the task it is passed still warns. **`mxcli test`** no longer prints MDL-DEPR001 / MDL-V1-SLASH warnings about the endpoint-registration script it generates itself: that script is `mdl 1`. **`check -p`** drops **MDL067** for a commit already stored the way the script writes it, as `exec` already did. **MDL-WIDGET15** skips a dynamictext with its own `class:` or `style:` (a laid-out label/value pair is not fused text), names the two widgets, and every widget-rule diagnostic now carries its page or snippet as its location instead of "(no module)". - **`page.title` and `catalog.pages.Title` are the project's default language** (mendixlabs/mxcli#1262) — the catalog took whichever translation of a page title a Go map range met first, so a lint rule reading `page.title` changed output between catalog rebuilds of an unchanged project. The title is now the project's default language, else en_US, else the lowest-sorted non-empty language; every translation stays available in `catalog.strings` (`StringContext = 'Forms$Page.Title'`). The catalog schema version moves to 17, so a cached catalog is rebuilt. - **Lint rules that read full-catalog data no longer pass silently** — `widgets()`, `xpath_expressions()`, `activities_for()`, `permissions()`, `permissions_for()` and a page's or snippet's `widget_count` read data only `refresh catalog full` writes, but only `refs_to` / `refs_from` raised the build depth, so unless another rule in the set happened to call `refs_to` the build stayed fast, they returned `[]` / `0` and the rule reported nothing (`widgets()`: 0 rows vs 46 on PedApp). They are now auto-detected like `refs_to`. The built-in MPR005, MPR006 and MPR012 read widgets the same way and were silent on a project without `.claude/lint-rules/` (an empty container went unreported); they now request the full build, so `mxcli lint` always builds a full catalog (measured on TestApp: 2.3 s -> 3.1 s for the build). From 6674cca371fafbcf1faac1e60dc6f761144abe57 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:04:29 +0000 Subject: [PATCH 08/13] docs(skills): move the drop + create advice into the microflow pitfalls write-microflows/SKILL.md is held to 700 lines; the advice to drop and create in one script lives in reference/pitfalls.md with a one-line link. Part of #944 (item 3). Co-Authored-By: Claude Opus 5.5 --- .claude/skills/mendix/write-microflows/SKILL.md | 8 +------- .../skills/mendix/write-microflows/reference/pitfalls.md | 7 +++++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.claude/skills/mendix/write-microflows/SKILL.md b/.claude/skills/mendix/write-microflows/SKILL.md index d2c8ce53ca..02f3844f2d 100644 --- a/.claude/skills/mendix/write-microflows/SKILL.md +++ b/.claude/skills/mendix/write-microflows/SKILL.md @@ -45,13 +45,7 @@ Choose the mode by who owns the microflow ([choose-edit-mode](../choose-edit-mod clause, `return` value, `if` condition, header clause, parameter (added/retyped; removed only if unused) or stated `@position`/`@start` change is patched in place (a move keeps the node's flows). A redrawn `@anchor`/`@curve`, loop body, error handler or other `return` added/taken away rebuilds under mdl 0 (`MDL-V1-REBUILD`: IDs - renumbered, merges and curves lost) and is refused under `mdl 1;`. **To change a loop body, `alter … replace` the whole loop** — neither mode edits inside one ([pitfalls](reference/pitfalls.md#11-changing-something-inside-a-loop-body)). -- **Rebuilding deliberately: `drop microflow X;` and `create microflow X …` in ONE script.** - The drop's module-role grants (and the unit's ID and folder) carry only to a create later in - the same script or REPL session. A drop in one run and a create in the next loses every - `grant execute` (CE0106 on the pages that call it); `drop` prints the roles it removed and - the `grant` to restore them. A `drop page` never carries its view grants. - + renumbered, merges and curves lost) and is refused under `mdl 1;`. **To change a loop body, `alter … replace` the whole loop** — neither mode edits inside one ([pitfalls](reference/pitfalls.md#11-changing-something-inside-a-loop-body)). **To rebuild deliberately, `drop` and `create` in ONE script** — the grants carry only within it ([pitfalls](reference/pitfalls.md#drop--create-is-still-a-new-document)). ## When to Use a Microflow vs a Nanoflow diff --git a/.claude/skills/mendix/write-microflows/reference/pitfalls.md b/.claude/skills/mendix/write-microflows/reference/pitfalls.md index a418e0a368..65540475ae 100644 --- a/.claude/skills/mendix/write-microflows/reference/pitfalls.md +++ b/.claude/skills/mendix/write-microflows/reference/pitfalls.md @@ -656,6 +656,13 @@ it from MDL. Nothing is lost by that — it only suppresses an editor warning. ## `drop` + `create` is still a new document +**Do the drop and the create in ONE script.** The drop's module-role grants (and +the unit's ID and folder) carry only to a create later in the same script or REPL +session. A drop in one run and a create in the next loses every `grant execute` — +CE0106 on the pages that call the flow (`check -p` reports it as MDL-SEC21). +`drop` prints the roles it removed and the `grant` that restores them. A `drop +page` never carries its view grants. + `drop microflow` followed by `create microflow` starts from nothing, so it keeps none of these unless the script restates them. Use `create or modify` to edit a microflow that carries any of them — and note that `describe` now emits all From 37b67ee33fbc4c4a3c851f8b2329e5ba3d63e86f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:04:38 +0000 Subject: [PATCH 09/13] fix(i18n): foresee CE4899 for tab page captions without the default language Measured on 11.14: of eleven caption kinds written en_US-only, a switch of the default to de_DE fails the build (CE4899 "Empty caption") only on tab page captions, in pages, snippets and layouts (not page templates or building blocks). - lint rule QUAL006 lists each one (error) - alter settings language (DefaultLanguageCode: ...) prints them - check -p reports MDL-I18N01 for a script that changes the default - the cached authoring language is dropped when the default changes, so a page created after the switch in the same script uses the new one Part of #944 (item 2). Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + cmd/mxcli/cmd_check.go | 5 + cmd/mxcli/cmd_lint.go | 1 + cmd/mxcli/syntax/features_misc.go | 15 +- docs-site/src/appendixes/error-messages.md | 14 ++ docs-site/src/tools/builtin-rules.md | 7 + mdl/executor/cmd_lint.go | 2 + mdl/executor/cmd_settings.go | 30 +++ mdl/executor/default_language_captions.go | 216 ++++++++++++++++++ .../default_language_captions_test.go | 164 +++++++++++++ mdl/linter/report.go | 1 + mdl/linter/rules/required_captions.go | 118 ++++++++++ mdl/linter/rules/required_captions_test.go | 79 +++++++ mdl/translations/required.go | 156 +++++++++++++ mdl/translations/required_test.go | 100 ++++++++ mdl/translations/sites.go | 16 +- 17 files changed, 914 insertions(+), 12 deletions(-) create mode 100644 mdl/executor/default_language_captions.go create mode 100644 mdl/executor/default_language_captions_test.go create mode 100644 mdl/linter/rules/required_captions.go create mode 100644 mdl/linter/rules/required_captions_test.go create mode 100644 mdl/translations/required.go create mode 100644 mdl/translations/required_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 32ceb70628..9ea31232a1 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -837,3 +837,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "describe prints a pluggable image's `Visible: Attr in (…)` (and expression Visible/Editable) twice", "cause": "the image branch appended appendConditionalProps and then appendAppearanceProps, which appends the same conditional settings", "file": "`mdl/executor/cmd_pages_describe_output.go` (image branch)", "insight": "appendAppearanceProps already owns visibility/editability; a branch that also appends them duplicates the key — grep for both on one widget kind", "refs": ["#721"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: describe → exec of TestApp's WorkflowCommons.UserTask_Assign fails: \"widget `grid8` (datagrid) cannot have its visibility set: its widget package declares no Visibility system property\"", "cause": "MDL-WIDGET41 gated Visible: on the package declaring , measured only from which packages declare it — but Studio Pro stores ConditionalVisibilitySettings on a Datagrid whose Type declares no Visibility property, and mxbuild 11.14 accepts static and conditional visibility there (0 errors)", "file": "`mdl/executor/pluggable_system_props.go` (`undeclaredSystemProps`)", "insight": "A declared system property is evidence of where a setting is shown, not of whether it exists; a refusal must be measured against what Studio Pro actually stores. Run the roundtrip harness over Studio Pro-authored pages before adding a refusal on stored shapes. Editability stays gated", "refs": []} {"date": "2026-10-03", "area": "mdl/executor/drop", "symptom": "`drop microflow M.F;` in one `mxcli exec` run and `create microflow M.F …` in the next leaves M.F with no module-role grants (CE0106 on the pages calling it); the drop printed only \"Dropped microflow: M.F\"", "cause": "the grants carry only through the session cache (rememberDroppedMicroflow / consumeDroppedMicroflow), which a later process does not have; nothing told the user the carry was session-scoped. `drop page` never remembers its AllowedRoles at all", "fix": "writeDroppedGrantsNote (mdl/executor/drop_grants_note.go), called from execDropMicroflow / execDropNanoflow / execDropPage, prints the removed roles, whether a create carries them (same script or session for flows; never for a page), and the `grant` that restores them; flowRefusal's rebuild advice says drop + create in the same script", "insight": "A carry that lives in a session cache is invisible at the statement that creates it; the place to say so is the drop, which is the last moment the roles are known. Snippets have no access roles, so they need nothing", "issue": "ako/mxcli#944", "file": "mdl/executor/drop_grants_note.go; cmd_microflows_drop.go; cmd_nanoflows_drop.go; cmd_pages_builder.go; flow_verdict.go", "test": "mdl/executor/drop_grants_note_test.go; flow_verdict_test.go (TestFlowRefusalNamesTheFlowAndTheReason)"} +{"date": "2026-10-03", "area": "mdl/executor/settings", "symptom": "after `alter settings language (DefaultLanguageCode: 'de_DE')`, `docker check` fails with CE4899 \"Empty caption. [German, Germany]\" at Tab page 'tabPage2' (Administration.Account_Overview, en_US only) while `check -p --references`, `lint` and exec are silent; a page created AFTER the switch in the same script fails the same way", "cause": "nothing compared required captions with DefaultLanguageCode (QUAL005 compares languages with each other, and `mxcli lint` does not even run it); and describeDefaultLanguage cached the authoring language once per session, so the switch did not reach later creates", "fix": "translations.MissingRequiredCaptions (measured set: Forms$TabPage.Caption in pages, snippets, layouts; templates and building blocks skipped) feeds lint QUAL006, the note printed by alterSettings (defaultLanguageChanged, which also drops the cached authoring language) and check -p MDL-I18N01 (CheckDefaultLanguageCaptions simulates which documents the script writes before/after the switch)", "insight": "Measure which caption kinds the build requires before flagging: of eleven kinds written en_US-only, only the tab page caption failed; flagging the rest would have made an error rule wrong ten times out of eleven. The first lint run also flagged 22 page-template tab pages mxbuild never reported, caught only by comparing lint's count with docker check's (1 vs 1 after the fix, 3 vs 3 on the e2e script)", "issue": "ako/mxcli#944", "file": "mdl/translations/required.go; mdl/linter/rules/required_captions.go; mdl/executor/default_language_captions.go; mdl/executor/cmd_settings.go (defaultLanguageChanged)", "test": "mdl/translations/required_test.go; mdl/linter/rules/required_captions_test.go; mdl/executor/default_language_captions_test.go"} diff --git a/CHANGELOG.md b/CHANGELOG.md index ce361291af..352c41091e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A required caption with no text in the default language is foreseen** (ako/mxcli#944) — making de_DE the default left a stock app's `Administration.Account_Overview` `tabPage2` (en_US only) empty in de_DE, and mxbuild refused it with CE4899 "Empty caption. [German, Germany]" while `check -p`, `lint` and `exec` said nothing. Measured on 11.14, the tab page caption is the one caption kind the build requires (page titles, buttons, labels, group boxes, column headers, menu items, enumeration captions and messages build without it; page templates and building blocks are not checked). Now: **lint rule QUAL006** (error) lists every tab page caption without the default language; **`alter settings language (DefaultLanguageCode: …)`** prints how many there are and where, with the `alter page … { set (Caption: …) on … }` that fixes one; **`check -p` reports MDL-I18N01** for a script that changes the default — stored captions, and captions the script wrote before the change. And a page created **after** the change in the same script is now written in the new default: the authoring language was resolved once per session, so it was still written in the old one and failed the build too. - **`drop microflow`, `drop nanoflow` and `drop page` name the module-role grants they remove** (ako/mxcli#944) — a drop printed only "Dropped microflow", so a drop in one run and a create in the next silently lost every `grant execute` (CapTrack hit CE0106). The drop now lists the roles, says a create later in the same script or session carries them (a microflow or nanoflow; a page never carries), and prints the `grant` that restores them. The `create or modify` splice refusal and the write-microflows / write-nanoflows skills say to do the drop and the create in one script. - **`theme create --from` names a seeded font family it does not ship** (ako/mxcli#944) — a design whose `--mxt-font`, `--mxt-font-heading` or `--mxt-font-mono` is led by a family the base theme does not vendor (Inter, say) got no `@font-face` and no file, and nothing was printed, so the theme rendered in a fallback font wherever the family was not installed. `theme create` now prints a note per such family, naming where to add its woff2 files and `@font-face`. A stack led by a generic family (`system-ui`, `sans-serif`, …) is not named. - **`page.title` and `catalog.pages.Title` are the project's default language** (mendixlabs/mxcli#1262) — the catalog took whichever translation of a page title a Go map range met first, so a lint rule reading `page.title` changed output between catalog rebuilds of an unchanged project. The title is now the project's default language, else en_US, else the lowest-sorted non-empty language; every translation stays available in `catalog.strings` (`StringContext = 'Forms$Page.Title'`). The catalog schema version moves to 17, so a cached catalog is rebuilt. diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 504d186656..2a79bc2fe8 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -397,6 +397,11 @@ func runCheckFile(cmd *cobra.Command, filePath string) int { // reported shape was drop + create in separate runs, which loses the // roles that `create or modify` keeps. projectViolations = append(projectViolations, exec.CheckFlowAccess(prog)...) + // MDL-I18N01 (MxBuild CE4899): a script that changes the default + // language leaves every required caption written in the old one empty + // in the new one (ako/mxcli#944). + projectViolations = append(projectViolations, exec.CheckDefaultLanguageCaptions(prog)...) + if len(projectViolations) > 0 { if isStructured { structured = append(structured, projectViolations...) diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go index f2ff170aad..c1d41be4c1 100644 --- a/cmd/mxcli/cmd_lint.go +++ b/cmd/mxcli/cmd_lint.go @@ -419,6 +419,7 @@ func builtinLintRules() []linter.Rule { rules.NewEmptyContainerRule(), rules.NewGallerySelectionListenerRule(), rules.NewDataViewLayoutGridRule(), + rules.NewRequiredCaptionDefaultLanguageRule(), // QUAL006 - CE4899, reads the stored units rules.NewPageNavigationSecurityRule(), rules.NewNoEntityAccessRulesRule(), rules.NewWeakPasswordPolicyRule(), diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 6c5778a264..0358269c4d 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -620,12 +620,15 @@ ALTER SETTINGS LANGUAGE DROP 'de_DE'; -- SET THE DEFAULT LANGUAGE BEFORE AUTHORING CONTENT. DefaultLanguageCode is not -- only the fallback — it is the language a new caption is STORED under, because -- Mendix has no language-neutral text. Creating a page and THEN switching the --- default leaves that page's texts in the old language, and nothing reports it: --- mx check is 0 errors either way and the symptom appears only in Studio Pro, as --- the empty-caption placeholder plus a "no translation for this language" --- warning. Recovery is to re-run the create statements; the texts are then --- written under the new default. CREATE TRANSLATIONS FOR the default is refused — --- it is the source language, not a translation target. +-- default leaves that page's texts in the old language. Most captions then show +-- in Studio Pro as the empty-caption placeholder with a "no translation for this +-- language" warning, and build anyway; a TAB PAGE caption does not — mxbuild +-- refuses it with CE4899 "Empty caption". Changing the default prints how many +-- required captions lack it, check -p reports them for a script that changes it +-- (MDL-I18N01), and mxcli lint lists them (QUAL006). Recovery is to re-run the +-- create statements (the texts are then written under the new default) or +-- ALTER PAGE P { SET (Caption: '…') ON tabPage1; };. CREATE TRANSLATIONS FOR the +-- default is refused — it is the source language, not a translation target. -- A language is identified by its CODE alone: Studio Pro's "Arabic, Sudan" is -- derived from ar_SD for display and is not stored in the model. diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index 026dc0225e..1662742615 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -175,6 +175,20 @@ level Production. This script creates it as a NEW microflow, ... [MDL-SEC21] It is an **error at security level Prototype or Production** (the stored level, or the one the script sets) and a warning at Off, where MxBuild does not check it. Only what the script changes is reported: a flow that already had the problem before the script, and that the script does not touch, is left to `mxcli docker check`. +### MDL-I18N01: A tab page caption without the default language (CE4899) + +``` +✗ Administration.Account_Overview: tab page caption tabPage2 ("Local Users") has no +de_DE text once this script makes de_DE the default language — mxbuild reports +CE4899 "Empty caption" [MDL-I18N01] + → change the default language before creating the document, or give it a de_DE + text after the change: `alter page Administration.Account_Overview { set (Caption: '…') on tabPage2; };` +``` + +**Cause:** With a project (`check -p`), the script changes `DefaultLanguageCode` while a tab page caption has no text in the new default: one stored in the project, or one a `create page` / `create snippet` earlier in the script wrote in the old default. Mendix has no language-neutral text — a caption is stored per language, and a new one under the default at the time. Measured on Mendix 11.14, a tab page caption is the one caption kind MxBuild refuses when the default has no text (CE4899 "Empty caption. [German, Germany]"); a page title, button, label or menu item caption builds and shows the placeholder in Studio Pro instead. A stock app's `Administration.Account_Overview` has an en_US-only `tabPage2`, so switching such an app to another default fails the build. + +**Solution:** Change the default language before the script creates its pages, or give each caption a text in the new default after the change — `alter page … { set (Caption: '…') on ; };` (or `alter snippet`) writes the default language. `alter settings language (DefaultLanguageCode: …)` prints the list when it runs, and `mxcli lint` reports the project's existing ones as **QUAL006**. A page the script creates or rewrites *after* the change is written in the new default and is not reported. + ### MDL-DUPDEF: Element defined twice in one script ``` diff --git a/docs-site/src/tools/builtin-rules.md b/docs-site/src/tools/builtin-rules.md index 3fb4137804..73b8629ff3 100644 --- a/docs-site/src/tools/builtin-rules.md +++ b/docs-site/src/tools/builtin-rules.md @@ -16,6 +16,12 @@ The **lint rules** below run with `mxcli lint`. There is also a separate group o | **MDL006** | Empty containers -- Detects container widgets with no children | | **MDL007** | Page navigation security -- Checks that pages called from microflows have appropriate access rules | +## Quality Rules + +| Rule | Description | +|------|-------------| +| **QUAL006** | Required caption without the default language -- A tab page caption (page, snippet or layout) with no text in the project's `DefaultLanguageCode`. MxBuild refuses it with CE4899 "Empty caption. [German, Germany]". Typically left behind by changing the default language after the pages were written. Error. Measured on Mendix 11.14: of the caption kinds tried (page title, group box, buttons, column header, label, dynamic text, title, enumeration value, menu item, message) only the tab page caption is required; page templates and building blocks are not checked | + ## Security Rules | Rule | Description | @@ -63,6 +69,7 @@ These rules run with `mxcli check` (and the LSP, for real-time diagnostics) rath | **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-I18N01** | `mxcli check -p` | A script that changes `DefaultLanguageCode` leaves a tab page caption — stored in the project, or created by the script before the change — with no text in the new default. MxBuild reports CE4899 "Empty caption". See [Error Messages](../appendixes/error-messages.md#mdl-i18n01-a-tab-page-caption-without-the-default-language-ce4899). | | **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/executor/cmd_lint.go b/mdl/executor/cmd_lint.go index 2a56c6af7a..634db86eb5 100644 --- a/mdl/executor/cmd_lint.go +++ b/mdl/executor/cmd_lint.go @@ -37,6 +37,7 @@ func execLint(ctx *ExecContext, s *ast.LintStmt) error { rules.NewImageSourceRule(), rules.NewLegacyImageWidgetRule(), rules.NewMissingTranslationsRule(), + rules.NewRequiredCaptionDefaultLanguageRule(), rules.NewGallerySelectionListenerRule(), rules.NewDataViewLayoutGridRule(), } @@ -153,6 +154,7 @@ func listLintRules(ctx *ExecContext) error { lint.AddRule(rules.NewImageSourceRule()) lint.AddRule(rules.NewLegacyImageWidgetRule()) lint.AddRule(rules.NewMissingTranslationsRule()) + lint.AddRule(rules.NewRequiredCaptionDefaultLanguageRule()) lint.AddRule(rules.NewGallerySelectionListenerRule()) lint.AddRule(rules.NewDataViewLayoutGridRule()) diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index b89c18bf6c..c706538229 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -389,6 +389,9 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { } section := strings.ToLower(stmt.Section) + // Set when this statement changes DefaultLanguageCode; see + // defaultLanguageChanged. + newDefaultLanguage := "" // Resolve the qualified names this statement would write BEFORE writing them. // `mxcli check --references` runs the same functions, so exec refuses exactly @@ -561,7 +564,11 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { if err := validateLanguageCode(ps.Language, valStr); err != nil { return err } + if !strings.EqualFold(ps.Language.DefaultLanguageCode, valStr) { + newDefaultLanguage = valStr + } ps.Language.DefaultLanguageCode = valStr + default: return mdlerrors.NewUnsupported("unknown language setting: " + key) } @@ -609,9 +616,32 @@ func alterSettings(ctx *ExecContext, stmt *ast.AlterSettingsStmt) error { } ctx.reportWrite(section+" settings", "Updated %s settings", section) + if newDefaultLanguage != "" { + defaultLanguageChanged(ctx, newDefaultLanguage) + } return nil } +// defaultLanguageChanged runs after a write that changed DefaultLanguageCode +// (ako/mxcli#944). +// +// The cached authoring language is dropped first: it was resolved once per +// session, so a page created later in the SAME script was still written in the +// old default — measured, its tab page caption then failed the build with +// CE4899 in the new one. Then it reports the required captions the project now +// has no text for, which is the moment they break. +func defaultLanguageChanged(ctx *ExecContext, lang string) { + if ctx.Cache != nil { + ctx.Cache.defaultLangLoaded = false + describeDefaultLanguage(ctx) // republish the authoring language + } + missing, err := missingDefaultCaptions(ctx, lang) + if err != nil { + return + } + writeMissingDefaultCaptionsNote(ctx.Output, lang, missing) +} + // 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 diff --git a/mdl/executor/default_language_captions.go b/mdl/executor/default_language_captions.go new file mode 100644 index 0000000000..fc99af6495 --- /dev/null +++ b/mdl/executor/default_language_captions.go @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "fmt" + "io" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/translations" +) + +// defaultCaptionRule is check's report of a required caption the script leaves +// without text in the project's default language: mxbuild CE4899 "Empty +// caption. [German, Germany]" (ako/mxcli#944). Which captions are required is +// translations.RequiredCaptionKind — measured, tab page captions only. +const defaultCaptionRule = "MDL-I18N01" + +// documentCaption is a required caption with the qualified name of the +// document it sits in. +type documentCaption struct { + Document string // "Administration.Account_Overview" + translations.MissingCaption +} + +func (d documentCaption) String() string { return d.Document + ": " + d.MissingCaption.String() } + +// missingDefaultCaptions lists the project's required captions with no text in +// lang, sorted by document, each named by its qualified document name. +func missingDefaultCaptions(ctx *ExecContext, lang string) ([]documentCaption, error) { + missing, err := translations.MissingRequiredCaptions(ctx.Backend, lang) + if err != nil || len(missing) == 0 { + return nil, err + } + h, _ := getHierarchy(ctx) + out := make([]documentCaption, 0, len(missing)) + for _, m := range missing { + qn := m.UnitName + if h != nil { + if mod := h.GetModuleName(h.FindModuleID(m.ContainerID)); mod != "" { + qn = mod + "." + m.UnitName + } + } + out = append(out, documentCaption{Document: qn, MissingCaption: m}) + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Document < out[j].Document }) + return out, nil +} + +// maxListedCaptions caps how many captions a note lists inline. +const maxListedCaptions = 10 + +// writeMissingDefaultCaptionsNote is what `alter settings language +// (DefaultLanguageCode: …)` prints after changing the default: how many required +// captions have no text in it, and where they are. Changing the default is the +// moment they break — every caption written so far is in the old language — +// and nothing else said so: check, lint and exec were all silent until mxbuild +// reported CE4899. +func writeMissingDefaultCaptionsNote(w io.Writer, lang string, missing []documentCaption) { + if len(missing) == 0 { + return + } + fmt.Fprintf(w, "\nNote: %d required caption(s) have no %s text, and mxbuild refuses them "+ + "(CE4899 \"Empty caption\"):\n", len(missing), lang) + for i, m := range missing { + if i == maxListedCaptions { + fmt.Fprintf(w, " … and %d more (mxcli lint lists them all as QUAL006)\n", len(missing)-maxListedCaptions) + break + } + fmt.Fprintf(w, " %s\n", m) + } + fmt.Fprintf(w, "Give each a %s text — an alter writes the default language, e.g. %s — or re-run the "+ + "script that created the page; `create translations for` does not reach the default language.\n", + lang, captionFixStatement(missing[0].UnitType, missing[0].Document, missing[0].OwnerName)) +} + +// captionFixStatement is the statement that gives a tab page a caption in the +// default language — an alter writes the authoring language, which is the +// default. A layout has no alter, so it is pointed at Studio Pro. +func captionFixStatement(unitType, doc, tab string) string { + kind := "page" + switch unitType { + case "Forms$Snippet": + kind = "snippet" + case "Forms$Layout": + return "set the caption of " + tab + " in layout " + doc + " in Studio Pro" + } + if tab == "" { + tab = "" + } + return fmt.Sprintf("`alter %s %s { set (Caption: '…') on %s; };`", kind, doc, tab) +} + +// (e *Executor) CheckDefaultLanguageCaptions reports MDL-I18N01 for the script. +func (e *Executor) CheckDefaultLanguageCaptions(prog *ast.Program) []linter.Violation { + if e == nil { + return nil + } + return CheckDefaultLanguageCaptions(e.newExecContext(context.Background()), prog) +} + +// CheckDefaultLanguageCaptions foresees CE4899 for a script that changes the +// project's default language: every required caption stored in the project, or +// written by the script before the change, has text only in the languages it +// was written in, and a build in the new default refuses each one. +// +// Only what the script causes is reported. A script that does not change the +// default writes its captions in the default (authoringLanguage), so its own +// captions are fine and the project's existing gaps are lint's (QUAL006). A +// document the script writes AFTER the change is written in the new default +// and is not reported; one it drops is gone. +func CheckDefaultLanguageCaptions(ctx *ExecContext, prog *ast.Program) []linter.Violation { + if ctx == nil || prog == nil || !ctx.Connected() { + return nil + } + switchAt, newLang := -1, "" + for i, stmt := range prog.Statements { + if s, ok := stmt.(*ast.AlterSettingsStmt); ok && strings.EqualFold(s.Section, "language") { + if v, ok := s.Properties["DefaultLanguageCode"]; ok { + switchAt, newLang = i, settingsValueToString(v) + } + } + } + if switchAt < 0 || newLang == "" { + return nil + } + if ps, err := ctx.Backend.GetProjectSettings(); err == nil && ps != nil && ps.Language != nil && + strings.EqualFold(ps.Language.DefaultLanguageCode, newLang) { + return nil // not a change + } + + // What the script does to each document, in order: the last word wins. + rewritten := map[string]bool{} // stored version is replaced or gone + written := map[string][]string{} // created before the switch: its tab pages + writtenType := map[string]string{} // a created snippet's unit type, for the fix statement + for i, stmt := range prog.Statements { + var name string + var widgets []*ast.WidgetV3 + isCreate := false + switch s := stmt.(type) { + case *ast.CreatePageStmtV3: + name, widgets, isCreate = s.Name.String(), s.Widgets, true + case *ast.CreateSnippetStmtV3: + name, widgets, isCreate = s.Name.String(), s.Widgets, true + writtenType[name] = "Forms$Snippet" + case *ast.DropPageStmt: + name = s.Name.String() + case *ast.DropSnippetStmt: + name = s.Name.String() + case *ast.AlterPageStmt: + if i < switchAt { + continue // an edit before the switch writes the old default + } + name = s.PageName.String() + default: + continue + } + rewritten[name] = true + delete(written, name) + if isCreate && i < switchAt { + written[name] = tabPageNames(widgets) + } + } + + var out []linter.Violation + add := func(unitType, doc, tab, what string) { + qn := splitQualifiedName(doc) + out = append(out, linter.Violation{ + RuleID: defaultCaptionRule, + Severity: linter.SeverityError, + Message: fmt.Sprintf("%s: %s has no %s text once this script makes %s the default language — "+ + "mxbuild reports CE4899 \"Empty caption\"", doc, what, newLang, newLang), + Suggestion: fmt.Sprintf("change the default language before creating the document, or give it a %s "+ + "text after the change: %s", newLang, captionFixStatement(unitType, doc, tab)), + Location: linter.Location{Module: qn.Module, DocumentName: qn.Name}, + }) + } + + stored, _ := missingDefaultCaptions(ctx, newLang) + for _, m := range stored { + if rewritten[m.Document] { + continue + } + add(m.UnitType, m.Document, m.OwnerName, m.MissingCaption.String()) + } + var names []string + for n := range written { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + for _, tab := range written[n] { + add(writtenType[n], n, tab, "tab page caption "+tab+" (written by this script before the change)") + } + } + return out +} + +// tabPageNames lists the tab pages in a widget tree, in document order. +func tabPageNames(ws []*ast.WidgetV3) []string { + var out []string + for _, w := range ws { + if w == nil { + continue + } + if strings.EqualFold(w.Type, "tabpage") { + out = append(out, w.Name) + } + out = append(out, tabPageNames(w.Children)...) + } + return out +} diff --git a/mdl/executor/default_language_captions_test.go b/mdl/executor/default_language_captions_test.go new file mode 100644 index 0000000000..b1dcd4b820 --- /dev/null +++ b/mdl/executor/default_language_captions_test.go @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// ako/mxcli#944: making de_DE the default left Administration.Account_Overview's +// en_US-only tabPage2 caption empty in de_DE, and mxbuild refused it with CE4899 +// while check, lint and exec said nothing. Measured on 11.14: tab page captions +// are the one caption kind the build requires. + +// tabPageUnit is a stored page whose one tab page caption has text only in the +// given languages. +func tabPageUnit(t *testing.T, pageName, tabName string, langs ...string) []byte { + t.Helper() + items := bson.A{int32(3)} + for _, l := range langs { + items = append(items, bson.D{ + {Key: "$Type", Value: "Texts$Translation"}, + {Key: "LanguageCode", Value: l}, + {Key: "Text", Value: "Local Users"}, + }) + } + raw, err := bson.Marshal(bson.D{ + {Key: "$ID", Value: "page-" + pageName}, + {Key: "$Type", Value: "Forms$Page"}, + {Key: "Name", Value: pageName}, + {Key: "Widgets", Value: bson.A{bson.D{ + {Key: "$ID", Value: "tab-" + tabName}, + {Key: "$Type", Value: "Forms$TabPage"}, + {Key: "Name", Value: tabName}, + {Key: "Caption", Value: bson.D{{Key: "$Type", Value: "Texts$Text"}, {Key: "Items", Value: items}}}, + }}}, + }) + if err != nil { + t.Fatal(err) + } + return raw +} + +// languageProject is a mock project with en_US and de_DE enabled, default +// en_US, holding Administration.Account_Overview with an en_US-only tabPage2. +func languageProject(t *testing.T) (*ExecContext, *mockLanguageState) { + t.Helper() + mod := mkModule("Administration") + st := &mockLanguageState{ps: &model.ProjectSettings{Language: &model.LanguageSettings{ + DefaultLanguageCode: "en_US", + Languages: []model.Language{{Code: "en_US"}, {Code: "de_DE"}}, + }}} + raw := tabPageUnit(t, "Account_Overview", "tabPage2", "en_US") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return st.ps, nil }, + UpdateProjectSettingsFunc: func(ps *model.ProjectSettings) error { + st.ps = ps + return nil + }, + ListUnitsFunc: func() ([]*types.UnitInfo, error) { + return []*types.UnitInfo{{ID: "u1", ContainerID: mod.ID, Type: "Forms$Page"}}, nil + }, + GetRawUnitBytesFunc: func(id model.ID) ([]byte, error) { return raw, nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + st.out = buf + return ctx, st +} + +type mockLanguageState struct { + ps *model.ProjectSettings + out interface{ String() string } +} + +func switchDefault(lang string) *ast.AlterSettingsStmt { + return &ast.AlterSettingsStmt{Section: "LANGUAGE", Properties: map[string]any{"DefaultLanguageCode": lang}} +} + +// (c) The change of default is when the captions break, so it says so. +func TestAlterDefaultLanguage_ReportsRequiredCaptionsWithoutIt(t *testing.T) { + ctx, st := languageProject(t) + assertNoError(t, alterSettings(ctx, switchDefault("de_DE"))) + out := st.out.String() + assertContainsStr(t, out, "1 required caption(s) have no de_DE text") + assertContainsStr(t, out, `Administration.Account_Overview: tab page caption tabPage2 ("Local Users")`) + assertContainsStr(t, out, "alter page Administration.Account_Overview { set (Caption: '…') on tabPage2; };") +} + +// Control: switching to a language the caption has says nothing. +func TestAlterDefaultLanguage_SilentWhenCaptionsHaveIt(t *testing.T) { + ctx, st := languageProject(t) + st.ps.Language.DefaultLanguageCode = "de_DE" + assertNoError(t, alterSettings(ctx, switchDefault("en_US"))) + if strings.Contains(st.out.String(), "required caption") { + t.Errorf("every tab page has en_US; want no note, got:\n%s", st.out.String()) + } +} + +// (b) The authoring language was resolved once per session, so a page created +// after the switch in the same script was still written in the old default — +// measured: its tab page then failed the de_DE build with CE4899 too. +func TestAlterDefaultLanguage_NewTextsUseTheNewDefault(t *testing.T) { + ctx, _ := languageProject(t) + if got := authoringLanguage(ctx); got != "en_US" { + t.Fatalf("before: authoringLanguage = %q", got) + } + assertNoError(t, alterSettings(ctx, switchDefault("de_DE"))) + if got := authoringLanguage(ctx); got != "de_DE" { + t.Errorf("after the switch: authoringLanguage = %q, want de_DE", got) + } +} + +func tabPageWidget(name string) *ast.WidgetV3 { + return &ast.WidgetV3{Type: "TABCONTAINER", Name: "tc", Children: []*ast.WidgetV3{ + {Type: "TABPAGE", Name: name, Properties: map[string]any{"Caption": "Tab"}}, + }} +} + +// (b) check -p foresees CE4899 for a script that changes the default: the +// stored page, and a page the script creates before the change. A page created +// after it is written in the new default and is fine. +func TestCheckDefaultLanguageCaptions(t *testing.T) { + ctx, _ := languageProject(t) + before := &ast.CreatePageStmtV3{Name: ast.QualifiedName{Module: "M", Name: "Before"}, Widgets: []*ast.WidgetV3{tabPageWidget("tpBefore")}} + after := &ast.CreatePageStmtV3{Name: ast.QualifiedName{Module: "M", Name: "After"}, Widgets: []*ast.WidgetV3{tabPageWidget("tpAfter")}} + prog := &ast.Program{Statements: []ast.Statement{before, switchDefault("de_DE"), after}} + + vs := CheckDefaultLanguageCaptions(ctx, prog) + var msgs []string + for _, v := range vs { + if v.RuleID != defaultCaptionRule { + t.Errorf("rule = %s", v.RuleID) + } + msgs = append(msgs, v.Message) + } + all := strings.Join(msgs, "\n") + if len(vs) != 2 || !strings.Contains(all, "Administration.Account_Overview: tab page caption tabPage2") || + !strings.Contains(all, "M.Before: tab page caption tpBefore") || strings.Contains(all, "tpAfter") { + t.Errorf("want the stored tabPage2 and tpBefore, not tpAfter; got:\n%s", all) + } + + // Control: the same script without the switch reports nothing. + prog = &ast.Program{Statements: []ast.Statement{before, after}} + if vs := CheckDefaultLanguageCaptions(ctx, prog); len(vs) != 0 { + t.Errorf("no default change; want nothing, got %+v", vs) + } + + // A stored page the script rewrites after the switch is written in the new + // default and is not reported. + rewrite := &ast.CreatePageStmtV3{Name: ast.QualifiedName{Module: "Administration", Name: "Account_Overview"}, + Widgets: []*ast.WidgetV3{tabPageWidget("tabPage2")}} + prog = &ast.Program{Statements: []ast.Statement{switchDefault("de_DE"), rewrite}} + if vs := CheckDefaultLanguageCaptions(ctx, prog); len(vs) != 0 { + t.Errorf("rewritten after the switch; want nothing, got %+v", vs) + } +} diff --git a/mdl/linter/report.go b/mdl/linter/report.go index a49fc8015c..4337046890 100644 --- a/mdl/linter/report.go +++ b/mdl/linter/report.go @@ -63,6 +63,7 @@ var categoryMapping = map[string]string{ "QUAL003": "Quality", "QUAL004": "Quality", "QUAL005": "Quality", + "QUAL006": "Quality", "CONV009": "Quality", "CONV012": "Quality", "CONV014": "Quality", diff --git a/mdl/linter/rules/required_captions.go b/mdl/linter/rules/required_captions.go new file mode 100644 index 0000000000..07465ee8a7 --- /dev/null +++ b/mdl/linter/rules/required_captions.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/translations" + "github.com/mendixlabs/mxcli/model" +) + +// RequiredCaptionDefaultLanguageRule reports a caption mxbuild requires in the +// project's default language and that has no text there: CE4899 "Empty +// caption. [German, Germany]" (ako/mxcli#944). +// +// QUAL005 cannot see it: it compares the languages texts are translated into +// with each other and never asks which one is the default, so a project whose +// default changed to de_DE after its pages were written in en_US is reported +// as nothing at all — or as a warning among hundreds — while the build fails. +// Which captions mxbuild requires is measured (translations.RequiredCaptionKind: +// tab page captions only), so this rule is an error and flags nothing else. +type RequiredCaptionDefaultLanguageRule struct{} + +// NewRequiredCaptionDefaultLanguageRule creates the rule. +func NewRequiredCaptionDefaultLanguageRule() *RequiredCaptionDefaultLanguageRule { + return &RequiredCaptionDefaultLanguageRule{} +} + +func (r *RequiredCaptionDefaultLanguageRule) ID() string { return "QUAL006" } +func (r *RequiredCaptionDefaultLanguageRule) Name() string { + return "RequiredCaptionMissingDefaultLanguage" +} +func (r *RequiredCaptionDefaultLanguageRule) Category() string { return "quality" } +func (r *RequiredCaptionDefaultLanguageRule) DefaultSeverity() linter.Severity { + return linter.SeverityError +} + +func (r *RequiredCaptionDefaultLanguageRule) Description() string { + return "Checks that every caption mxbuild requires (a tab page caption) has text in the project's default language (CE4899)" +} + +// captionProject is what the rule needs from the lint reader: the stored units +// and the project's default language. The backend behind `mxcli lint` has both; +// a reader without them (a test double, no project) makes the rule silent. +type captionProject interface { + translations.UnitReader + GetProjectSettings() (*model.ProjectSettings, error) +} + +// Check runs the rule. +func (r *RequiredCaptionDefaultLanguageRule) Check(ctx *linter.LintContext) []linter.Violation { + p, ok := ctx.Reader().(captionProject) + if !ok || p == nil { + return nil + } + ps, err := p.GetProjectSettings() + if err != nil || ps == nil || ps.Language == nil || ps.Language.DefaultLanguageCode == "" { + return nil + } + lang := ps.Language.DefaultLanguageCode + missing, err := translations.MissingRequiredCaptions(p, lang) + if err != nil { + return nil + } + return r.violations(ctx, lang, missing) +} + +// violations names each missing caption by its document, resolved through the +// catalog's objects view, and applies the module and document filters. +func (r *RequiredCaptionDefaultLanguageRule) violations(ctx *linter.LintContext, lang string, missing []translations.MissingCaption) []linter.Violation { + var out []linter.Violation + for _, m := range missing { + qn, name, module, docType := m.UnitName, m.UnitName, "", "page" + if db := ctx.CatalogDB(); db != nil { + row := db.QueryRow(`SELECT QualifiedName, Name, ModuleName, ObjectType FROM objects WHERE Id = ?`, string(m.UnitID)) + var q, n, mod, ot string + if row.Scan(&q, &n, &mod, &ot) == nil { + qn, name, module, docType = q, n, mod, strings.ToLower(ot) + } + } + if module != "" && ctx.IsExcluded(module) { + continue + } + if ctx.IsDocumentExcluded(qn) { + continue + } + out = append(out, linter.Violation{ + RuleID: r.ID(), + Severity: r.DefaultSeverity(), + Message: fmt.Sprintf("%s: %s has no text in the default language %s — mxbuild reports CE4899 \"Empty caption\"", + qn, m.String(), lang), + Location: linter.Location{ + Module: module, + DocumentType: docType, + DocumentName: name, + DocumentID: string(m.UnitID), + }, + Suggestion: captionSuggestion(lang, m.UnitType, qn, m.OwnerName), + }) + } + return out +} + +// captionSuggestion names the statement that fixes it: an alter writes the +// authoring language, which is the default. A layout has no alter. +func captionSuggestion(lang, unitType, doc, tab string) string { + kind := "page" + switch unitType { + case "Forms$Snippet": + kind = "snippet" + case "Forms$Layout": + return fmt.Sprintf("Give %s a %s caption in Studio Pro (layout %s)", tab, lang, doc) + } + return fmt.Sprintf("Give it a %s text: alter %s %s { set (Caption: '…') on %s; }; writes the default language", + lang, kind, doc, tab) +} diff --git a/mdl/linter/rules/required_captions_test.go b/mdl/linter/rules/required_captions_test.go new file mode 100644 index 0000000000..b77d118c75 --- /dev/null +++ b/mdl/linter/rules/required_captions_test.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "database/sql" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/translations" + + _ "modernc.org/sqlite" +) + +func objectsDB(t *testing.T) catalog.CatalogDB { + t.Helper() + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatal(err) + } + if _, err := db.Exec(`CREATE TABLE objects (Id TEXT, QualifiedName TEXT, Name TEXT, ModuleName TEXT, ObjectType TEXT)`); err != nil { + t.Fatal(err) + } + if _, err := db.Exec(`INSERT INTO objects VALUES + ('u1', 'Administration.Account_Overview', 'Account_Overview', 'Administration', 'PAGE'), + ('u2', 'Shop.Header', 'Header', 'Shop', 'SNIPPET')`); err != nil { + t.Fatal(err) + } + return catalog.WrapSqlDB(db) +} + +// QUAL006 (ako/mxcli#944): a tab page caption with no text in the default +// language is the CE4899 the de_DE build reported and QUAL005 never did. Each +// one is an error named by its document, with the alter that fixes it. +func TestRequiredCaptionDefaultLanguage_NamesEachCaption(t *testing.T) { + db := objectsDB(t) + defer db.Close() + ctx := linter.NewLintContextFromDB(db) + r := NewRequiredCaptionDefaultLanguageRule() + missing := []translations.MissingCaption{ + {UnitID: "u1", UnitType: "Forms$Page", UnitName: "Account_Overview", Kind: "tab page caption", OwnerName: "tabPage2", Sample: "Local Users"}, + {UnitID: "u2", UnitType: "Forms$Snippet", UnitName: "Header", Kind: "tab page caption", OwnerName: "tpMain"}, + } + vs := r.violations(ctx, "de_DE", missing) + if len(vs) != 2 { + t.Fatalf("want 2, got %+v", vs) + } + v := vs[0] + if v.RuleID != "QUAL006" || v.Severity != linter.SeverityError { + t.Errorf("rule/severity = %s/%v", v.RuleID, v.Severity) + } + if !strings.Contains(v.Message, `Administration.Account_Overview: tab page caption tabPage2 ("Local Users") has no text in the default language de_DE`) { + t.Errorf("message = %s", v.Message) + } + if v.Location.Module != "Administration" || v.Location.DocumentName != "Account_Overview" || v.Location.DocumentType != "page" { + t.Errorf("location = %+v", v.Location) + } + if !strings.Contains(vs[1].Suggestion, "alter snippet Shop.Header { set (Caption: '…') on tpMain; }") { + t.Errorf("snippet suggestion = %s", vs[1].Suggestion) + } + + // The module filter applies. + ctx.SetExcludedModules([]string{"Shop"}) + if vs := r.violations(ctx, "de_DE", missing); len(vs) != 1 { + t.Errorf("Shop excluded; want 1, got %d", len(vs)) + } +} + +// Without a project reader (no units, no default language) the rule is silent +// rather than guessing a language. +func TestRequiredCaptionDefaultLanguage_NoReaderIsSilent(t *testing.T) { + db := objectsDB(t) + defer db.Close() + if vs := NewRequiredCaptionDefaultLanguageRule().Check(linter.NewLintContextFromDB(db)); len(vs) != 0 { + t.Errorf("got %+v", vs) + } +} diff --git a/mdl/translations/required.go b/mdl/translations/required.go new file mode 100644 index 0000000000..487ec3c9ce --- /dev/null +++ b/mdl/translations/required.go @@ -0,0 +1,156 @@ +// SPDX-License-Identifier: Apache-2.0 + +package translations + +import ( + "fmt" + "sort" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// requiredCaptions are the texts mxbuild refuses to build when the project's +// default language has no text for them: CE4899 "Empty caption. [German, +// Germany]" (ako/mxcli#944), keyed ".". +// +// Measured, not assumed — only flag what mxbuild flags. On a fresh 11.14.0 app +// a page title, a tab page caption, a group box caption, an action and a link +// button caption, a data grid column header, an input label, a dynamic text, a +// title widget, an enumeration value caption, a navigation menu item caption +// and a show-message text were each written with an en_US text only; the +// default language was then switched to de_DE (with en_US still enabled, and +// again with it dropped). `mx check` failed with CE4899 on the tab pages and on +// nothing else, both times — including the stock Administration.Account_Overview +// tabPage2. A caption kind belongs here only once a build has been seen to +// refuse it. +var requiredCaptions = map[string]string{ + "Forms$TabPage.Caption": "tab page caption", +} + +// RequiredCaptionKind names a site mxbuild requires in the default language, +// or "" for a text it does not require. +func RequiredCaptionKind(s Site) string { + return requiredCaptions[s.OwnerType+"."+s.Property] +} + +// MissingCaption is one required caption with no text in the language asked +// about. +type MissingCaption struct { + UnitID model.ID + ContainerID model.ID + UnitType string + // UnitName is the document's own Name; the caller qualifies it. + UnitName string + // Kind is the caption kind ("tab page caption"). + Kind string + OwnerName string + ElementID string + // Sample is the text in another language, for the message; "" when the + // caption is empty in every language. + Sample string +} + +// String is "tab page caption tabPage2 (\"Local Users\")". +func (m MissingCaption) String() string { + s := m.Kind + if m.OwnerName != "" { + s += " " + m.OwnerName + } + if m.Sample != "" { + s += fmt.Sprintf(" (%q)", m.Sample) + } + return s +} + +// UnitReader is what MissingRequiredCaptions reads: the units and their bytes. +type UnitReader interface { + ListUnits() ([]*types.UnitInfo, error) + GetRawUnitBytes(id model.ID) ([]byte, error) +} + +// MissingRequiredCaptions lists every required caption with no non-empty text +// in lang — the CE4899 set a build in that default language will report. +// Excluded documents are skipped: mxbuild does not check them. +func MissingRequiredCaptions(p UnitReader, lang string) ([]MissingCaption, error) { + units, err := p.ListUnits() + if err != nil { + return nil, fmt.Errorf("list units: %w", err) + } + var out []MissingCaption + for _, u := range units { + if u == nil { + continue + } + raw, err := p.GetRawUnitBytes(u.ID) + if err != nil || len(raw) == 0 { + continue + } + for _, m := range MissingRequiredCaptionsInUnit(u.ID, u.Type, raw, lang) { + m.ContainerID = u.ContainerID + out = append(out, m) + } + } + return out, nil +} + +// checkedUnitTypes are the documents mxbuild checks for CE4899. A page template +// and a building block hold tab pages too — a stock 11.14 app has 22 of them +// with en_US-only captions in Atlas_Web_Content — but they are blueprints Studio +// Pro copies from, and the de_DE build that failed on the one page reported +// none of them. +var checkedUnitTypes = map[string]bool{ + "Forms$Page": true, + "Forms$Snippet": true, + "Forms$Layout": true, +} + +// MissingRequiredCaptionsInUnit is MissingRequiredCaptions over one unit. +func MissingRequiredCaptionsInUnit(id model.ID, unitType string, raw []byte, lang string) []MissingCaption { + if !checkedUnitTypes[unitType] { + return nil + } + var doc bson.D + + if err := bson.Unmarshal(raw, &doc); err != nil { + return nil + } + if excluded, _ := lookup(doc, "Excluded").(bool); excluded { + return nil + } + name, _ := lookup(doc, "Name").(string) + var out []MissingCaption + for _, s := range SitesIn(doc) { + kind := RequiredCaptionKind(s) + if kind == "" || s.Targets[lang] != "" { + continue + } + out = append(out, MissingCaption{ + UnitID: id, UnitType: unitType, UnitName: name, Kind: kind, + OwnerName: s.OwnerName, ElementID: s.ElementID, Sample: sampleText(s.Targets), + }) + } + return out +} + +// sampleText is a non-empty text from the lowest-sorting language, so a +// message names the caption the reader recognises. +func sampleText(targets map[string]string) string { + for _, l := range sortedLanguages(targets) { + if targets[l] != "" { + return targets[l] + } + } + return "" +} + +func sortedLanguages(m map[string]string) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} diff --git a/mdl/translations/required_test.go b/mdl/translations/required_test.go new file mode 100644 index 0000000000..59219776f2 --- /dev/null +++ b/mdl/translations/required_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 + +package translations + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// probePage is the measured case (ako/mxcli#944): a page whose texts are all +// en_US only. With de_DE the default, mxbuild reports CE4899 on the tab page +// caption and on nothing else, so the tab page is the only one to report. +func probePage(t *testing.T, excluded bool, tabCaption bson.D) []byte { + t.Helper() + doc := bson.D{ + {Key: "$ID", Value: "page-id"}, + {Key: "$Type", Value: "Forms$Page"}, + {Key: "Name", Value: "Account_Overview"}, + {Key: "Excluded", Value: excluded}, + {Key: "Title", Value: text(tr("en_US", "Accounts"))}, + {Key: "Widgets", Value: bson.A{ + bson.D{ + {Key: "$ID", Value: "tab-id"}, + {Key: "$Type", Value: "Forms$TabPage"}, + {Key: "Name", Value: "tabPage2"}, + {Key: "Caption", Value: tabCaption}, + }, + bson.D{ + {Key: "$ID", Value: "btn-id"}, + {Key: "$Type", Value: "Forms$ActionButton"}, + {Key: "Name", Value: "btn1"}, + {Key: "Caption", Value: text(tr("en_US", "Save"))}, + }, + }}, + } + raw, err := bson.Marshal(doc) + if err != nil { + t.Fatal(err) + } + return raw +} + +func TestMissingRequiredCaptions_OnlyTheTabPageCaption(t *testing.T) { + raw := probePage(t, false, text(tr("en_US", "Local Users"), tr("nl_NL", "Lokale gebruikers"))) + + got := MissingRequiredCaptionsInUnit("u1", "Forms$Page", raw, "de_DE") + if len(got) != 1 { + t.Fatalf("want exactly the tab page (page title and button caption are not required), got %+v", got) + } + m := got[0] + if m.Kind != "tab page caption" || m.OwnerName != "tabPage2" || m.UnitName != "Account_Overview" || + m.Sample != "Local Users" { + t.Errorf("got %+v", m) + } + if s := m.String(); s != `tab page caption tabPage2 ("Local Users")` { + t.Errorf("String() = %s", s) + } + + // Control: the same page in its own default language reports nothing. + if got := MissingRequiredCaptionsInUnit("u1", "Forms$Page", raw, "en_US"); len(got) != 0 { + t.Errorf("en_US is present; want nothing, got %+v", got) + } +} + +func TestMissingRequiredCaptions_EmptyTextCountsAsMissing(t *testing.T) { + raw := probePage(t, false, text(tr("de_DE", ""), tr("en_US", "Local Users"))) + if got := MissingRequiredCaptionsInUnit("u1", "Forms$Page", raw, "de_DE"); len(got) != 1 { + t.Errorf("an empty de_DE text is the CE4899 case; got %+v", got) + } + none := probePage(t, false, text()) + got := MissingRequiredCaptionsInUnit("u1", "Forms$Page", none, "de_DE") + if len(got) != 1 || got[0].Sample != "" { + t.Errorf("a caption with no text at all is missing too; got %+v", got) + } +} + +func TestMissingRequiredCaptions_SkipsExcludedDocuments(t *testing.T) { + raw := probePage(t, true, text(tr("en_US", "Local Users"))) + if got := MissingRequiredCaptionsInUnit("u1", "Forms$Page", raw, "de_DE"); len(got) != 0 { + t.Errorf("mxbuild does not check an excluded document; got %+v", got) + } +} + +// A page template and a building block are blueprints: the de_DE build that +// failed on Account_Overview reported none of the 22 en_US-only tab pages in +// Atlas_Web_Content's templates. Control: the same bytes as a page are reported. +func TestMissingRequiredCaptions_SkipsTemplates(t *testing.T) { + raw := probePage(t, false, text(tr("en_US", "Tab 1"))) + for _, ty := range []string{"Forms$PageTemplate", "Forms$BuildingBlock"} { + if got := MissingRequiredCaptionsInUnit("u1", ty, raw, "de_DE"); len(got) != 0 { + t.Errorf("%s is not built; got %+v", ty, got) + } + } + for _, ty := range []string{"Forms$Page", "Forms$Snippet"} { + if got := MissingRequiredCaptionsInUnit("u1", ty, raw, "de_DE"); len(got) != 1 { + t.Errorf("%s: want the tab page, got %+v", ty, got) + } + } +} diff --git a/mdl/translations/sites.go b/mdl/translations/sites.go index 50ce37b5e7..b343ac462d 100644 --- a/mdl/translations/sites.go +++ b/mdl/translations/sites.go @@ -30,6 +30,8 @@ type Site struct { // an enumeration's twelve values into one and calls the set complete as soon // as any one value is translated. ElementID string + // OwnerName is the owner's Name ("tabPage2"), empty when it has none. + OwnerName string // Targets is language code → text, exactly as stored. A language present // with an empty string is a text that exists but is not translated yet. Targets map[string]string @@ -39,8 +41,8 @@ type Site struct { // order. Order is stable so a caller writing rows gets a deterministic result. func SitesIn(doc bson.D) []Site { var out []Site - var walk func(v any, ownerType, ownerID, prop string) - walk = func(v any, ownerType, ownerID, prop string) { + var walk func(v any, ownerType, ownerID, ownerName, prop string) + walk = func(v any, ownerType, ownerID, ownerName, prop string) { switch n := v.(type) { case bson.D: if ty, _ := lookup(n, "$Type").(string); ty == "Texts$Text" { @@ -48,6 +50,7 @@ func SitesIn(doc bson.D) []Site { OwnerType: ownerType, Property: prop, ElementID: ownerID, + OwnerName: ownerName, Targets: translationsOf(n), }) return @@ -56,23 +59,24 @@ func SitesIn(doc bson.D) []Site { // it. A node without one (an anonymous sub-document) leaves the // owner as it was, so the text is still attributed to a real // element rather than to nothing. - nt, nid := ownerType, ownerID + nt, nid, nname := ownerType, ownerID, ownerName if ty, _ := lookup(n, "$Type").(string); ty != "" { nt = ty nid = elementIDOf(n) + nname, _ = lookup(n, "Name").(string) } for _, e := range n { - walk(e.Value, nt, nid, e.Key) + walk(e.Value, nt, nid, nname, e.Key) } case bson.A: // An array element inherits the property its array hangs off, so a // text inside `Widgets` is not reported as living at "Widgets". for _, e := range n { - walk(e, ownerType, ownerID, prop) + walk(e, ownerType, ownerID, ownerName, prop) } } } - walk(doc, "", "", "") + walk(doc, "", "", "", "") return out } From 33a92eda7884d173e951c6ab42eaa9850062ef98 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:07:57 +0000 Subject: [PATCH 10/13] fix(splice): spelling-only AST flags are no difference to the statement diff (#942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `commit … with events` never matched its stored bare commit: matchValue compared MfCommitStmt.ExplicitWithEvents, which only records that the redundant clause was written. An unchanged re-run re-spliced the commit, and a loop-body change next to it made a run of two that bypassed the loop-body refusal, rebuilding the loop with new $IDs under mdl 1. The audit found the same class in InheritanceSplitStmt.LegacyCaseKeyword / LegacyElseKeyword (skipped as spelling fields) and in an empty `else` (IfStmt.HasElse), which canonicalFlow now drops since describe never prints one. `without events` is still compared. Co-Authored-By: Claude Opus 5.5 --- mdl/executor/flow_canonical.go | 7 ++ mdl/executor/flow_declared_match.go | 20 +++ mdl/executor/flow_spelling_flags_test.go | 94 ++++++++++++++ .../flow_commit_events_rerun_test.go | 116 ++++++++++++++++++ 4 files changed, 237 insertions(+) create mode 100644 mdl/executor/flow_spelling_flags_test.go create mode 100644 mdl/roundtrip/flow_commit_events_rerun_test.go diff --git a/mdl/executor/flow_canonical.go b/mdl/executor/flow_canonical.go index 6d819a820e..8c59a3565a 100644 --- a/mdl/executor/flow_canonical.go +++ b/mdl/executor/flow_canonical.go @@ -23,6 +23,10 @@ import "github.com/mendixlabs/mxcli/mdl/ast" // form: the else statements follow the if in the enclosing list, where // each is still located by its @position and stays addressable. // +// - An `if` with an empty `else` and the same `if` with none. Both build a +// split whose false flow goes on to what follows; describe drops the +// empty else, so the canonical form has none (ako/mxcli#942). +// // - A `join L` directly followed by its own `merge L`, where nothing else // joins L. Falling through into a merge and joining it are the same flow, // and a merge only one path reaches is no join point at all: describe @@ -75,6 +79,9 @@ func canonicalNested(st ast.MicroflowStatement, joins map[string]int) ast.Microf case *ast.IfStmt: c := *s c.ThenBody, c.ElseBody = canonicalList(s.ThenBody, joins), canonicalList(s.ElseBody, joins) + if len(c.ElseBody) == 0 { + c.HasElse = false + } return &c case *ast.LoopStmt: c := *s diff --git a/mdl/executor/flow_declared_match.go b/mdl/executor/flow_declared_match.go index 7f17de2cdf..7cbddc3ea0 100644 --- a/mdl/executor/flow_declared_match.go +++ b/mdl/executor/flow_declared_match.go @@ -82,6 +82,22 @@ var geometryFields = map[reflect.Type]map[string]bool{ paramStructType: {"Position": true}, } +// spellingFields names, per AST type, the fields that record how a statement +// was SPELLED rather than what it stores. Describe prints one spelling, so the +// stored side never carries the other, and comparing these would make a +// declared statement fail to match its own stored activity (ako/mxcli#942): +// re-spliced on every run, and next to a loop the run of "changes" swallowed +// the loop and rebuilt it with new $IDs under mdl 1. They exist only for +// diagnostics (MDL067, MDL065) and nothing downstream branches on them. +var spellingFields = map[reflect.Type]map[string]bool{ + // `commit $X with events` is a bare `commit $X`: both store events on. + // WithoutEvents, which IS stored, is still compared. + reflect.TypeOf(ast.MfCommitStmt{}): {"ExplicitWithEvents": true}, + // `case X` / `else` are the legacy spellings of `when X then` / + // `when (empty) then`; both build the identical split. + reflect.TypeOf(ast.InheritanceSplitStmt{}): {"LegacyCaseKeyword": true, "LegacyElseKeyword": true}, +} + func matchValue(d, s reflect.Value, mode matchMode) bool { if !d.IsValid() || !s.IsValid() { return d.IsValid() == s.IsValid() @@ -133,9 +149,13 @@ func matchValue(d, s reflect.Value, mode matchMode) bool { d, s = messageAsTemplate(d), messageAsTemplate(s) } geo := geometryFields[d.Type()] + spelling := spellingFields[d.Type()] for i := 0; i < d.NumField(); i++ { df := d.Field(i) name := d.Type().Field(i).Name + if spelling[name] { + continue + } skip := mode == matchAnyLayout || (mode == matchAnyPosition && d.Type() == annotationsStructType && positionFields[name]) if geo[name] && df.Kind() == reflect.Pointer && (skip || df.IsNil()) { diff --git a/mdl/executor/flow_spelling_flags_test.go b/mdl/executor/flow_spelling_flags_test.go new file mode 100644 index 0000000000..2a627590b5 --- /dev/null +++ b/mdl/executor/flow_spelling_flags_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// ako/mxcli#942: an AST flag that records how a statement was SPELLED, not +// what it stores, is no difference to the statement diff. Describe prints one +// spelling, so comparing such a flag made a statement never match its own +// stored activity: re-spliced on every run, and next to a loop the run of +// changes swallowed the loop and rebuilt it with new $IDs under mdl 1. +func TestDeclaredMatches_SpellingOnlyFlagsAreNotADifference(t *testing.T) { + for _, c := range []struct { + name, stored, declared string + want bool + }{ + {"commit: with events is the stored bare commit", + ` commit $In;`, ` commit $In with events;`, true}, + {"commit: bare", ` commit $In;`, ` commit $In;`, true}, + {"control: commit without events is not the bare commit", + ` commit $In;`, ` commit $In without events;`, false}, + {"control: with events is not a stored without events", + ` commit $In without events;`, ` commit $In with events;`, false}, + {"control: refresh added", ` commit $In;`, ` commit $In with events refresh;`, false}, + {"split type: legacy case/else is the when form describe prints", + ` split type $In + when M.A then + log info node 'N' 'a'; + when (empty) then + log info node 'N' 'e'; + end split;`, + ` split type $In + case M.A + log info node 'N' 'a'; + else + log info node 'N' 'e'; + end split;`, true}, + {"control: split type with a branch changed", + ` split type $In + when M.A then + log info node 'N' 'a'; + when (empty) then + log info node 'N' 'e'; + end split;`, + ` split type $In + case M.A + log info node 'N' 'b'; + else + log info node 'N' 'e'; + end split;`, false}, + } { + t.Run(c.name, func(t *testing.T) { + declared := parseFlowBody(t, c.declared) + stored := parseFlowBody(t, c.stored) + if got := declaredMatches(declared.Body, stored.Body); got != c.want { + t.Errorf("declaredMatches = %v, want %v", got, c.want) + } + }) + } +} + +// An `else` with nothing in it builds the same split as no else: the false +// flow goes on to what follows. Describe drops the empty else, so the declared +// form must be canonicalised to match its own stored `if`. +func TestCanonicalFlow_EmptyElseIsNoElse(t *testing.T) { + stored := parseFlowBody(t, ` if $In = 'x' then + log info node 'N' 'x'; + end if; + log info node 'N' 'after';`) + for _, c := range []struct { + name, declared string + want bool + }{ + {"empty else", ` if $In = 'x' then + log info node 'N' 'x'; + else + end if; + log info node 'N' 'after';`, true}, + {"control: an else with a body", ` if $In = 'x' then + log info node 'N' 'x'; + else + log info node 'N' 'y'; + end if; + log info node 'N' 'after';`, false}, + } { + t.Run(c.name, func(t *testing.T) { + d := canonicalFlow(parseFlowBody(t, c.declared).Body) + s := canonicalFlow(stored.Body) + if got := declaredMatches(d, s); got != c.want { + t.Errorf("declaredMatches = %v, want %v", got, c.want) + } + }) + } +} diff --git a/mdl/roundtrip/flow_commit_events_rerun_test.go b/mdl/roundtrip/flow_commit_events_rerun_test.go new file mode 100644 index 0000000000..377513bc8b --- /dev/null +++ b/mdl/roundtrip/flow_commit_events_rerun_test.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build integration + +package roundtrip + +import ( + "fmt" + "strings" + "testing" +) + +// ako/mxcli#942: `commit $L with events` and a bare `commit $L` are one stored +// activity (events on), and describe prints the bare form. The statement diff +// compared the AST flag that records the redundant `with events` was written, +// so the declared commit never matched its own stored activity: +// +// - every re-exec of an unchanged script reported "Unchanged microflow … +// (spliced: 1 replaced)" — and replaced the commit; +// - a change inside a loop body next to such a commit made the diff run the +// loop and the commit together, which bypassed the "changes inside its +// body" refusal: under mdl 1 the loop was silently rebuilt with new $IDs. +// +// The bare-commit spelling is the control for both: it always behaved. +func commitEventsFlow(name, commit, value string) string { + return fmt.Sprintf(`create or modify microflow MyFirstModule.%s ($L: List of System.User) +begin + log info node 'N' 'start'; + loop $U in $L + begin + change $U (Name = '%s'); + end loop; + %s; +end; +`, name, value, commit) +} + +func TestSpliceRerun_CommitWithEventsIsTheStoredCommit(t *testing.T) { + h := newHarness(t) + defer h.close() + for _, c := range []struct{ name, flow, commit string }{ + {"with events", "Repro_CommitWithEvents", "commit $L with events"}, + {"control: bare commit", "Repro_CommitBare", "commit $L"}, + } { + t.Run(c.name, func(t *testing.T) { + h.restore() + script := "mdl 1;\n" + commitEventsFlow(c.flow, c.commit, "a") + if err := h.exec(script); err != nil { + t.Fatalf("create: %v\n%s", err, h.out.String()) + } + // A real change outside the loop is spliced: one statement. + edited := strings.Replace(script, "'start'", "'begin'", 1) + before := h.snapshot() + if err := h.exec(edited); err != nil { + t.Fatalf("change: %v\n%s", err, h.out.String()) + } + if !strings.Contains(h.out.String(), "(spliced: 1 replaced)") { + t.Errorf("change: want one statement replaced:\n%s", h.out.String()) + } + if len(before.diff(h.snapshot())) == 0 { + t.Fatal("change: the real change wrote nothing") + } + // Re-executing it writes nothing and splices nothing. + assertUnchangedRerun(t, h, edited) + // The same where the flow is drawn other than the builder would + // draw it — as Studio Pro, or a splice, leaves it — so that it is + // the statement diff that decides, not the built comparison: the + // commit moved, which a script without @position does not state. + drawn := strings.Replace(edited, " "+c.commit+";", " @position(900, 320)\n "+c.commit+";", 1) + if err := h.exec(drawn); err != nil { + t.Fatalf("move the commit: %v\n%s", err, h.out.String()) + } + if !strings.Contains(h.describeUnder("mdl 1;", "microflow MyFirstModule."+c.flow), "@position(900, 320)") { + t.Fatalf("the commit was not moved:\n%s", h.out.String()) + } + assertUnchangedRerun(t, h, edited) + + // A change inside the loop body is refused under mdl 1, nothing + // written — the commit next to the loop must not turn it into a + // replace of the loop. + stored := drawnObjects(t, h.flowUnit(t, c.flow)) + before = h.snapshot() + err := h.exec("mdl 1;\n" + strings.Replace(commitEventsFlow(c.flow, c.commit, "z"), "'start'", "'begin'", 1)) + if err == nil || !strings.Contains(err.Error(), "changes inside its body") { + t.Errorf("a loop-body change: want the refusal naming the loop, got %v\n%s", err, h.out.String()) + } + if changed := before.diff(h.snapshot()); len(changed) != 0 { + t.Errorf("the refused loop-body change wrote: %v\n%s", changed, h.out.String()) + } + after := drawnObjects(t, h.flowUnit(t, c.flow)) + for id := range stored { + if _, ok := after[id]; !ok { + t.Errorf("stored object %s was renumbered away", id) + } + } + }) + } +} + +// assertUnchangedRerun runs script twice: neither run may write or splice. +func assertUnchangedRerun(t *testing.T, h *harness, script string) { + t.Helper() + for run := 1; run <= 2; run++ { + before := h.snapshot() + if err := h.exec(script); err != nil { + t.Fatalf("re-run %d: %v\n%s", run, err, h.out.String()) + } + if changed := before.diff(h.snapshot()); len(changed) != 0 { + t.Errorf("re-run %d of the unchanged script wrote: %v\n%s", run, changed, h.out.String()) + } + if out := h.out.String(); strings.Contains(out, "spliced") || !strings.Contains(out, "Unchanged microflow") { + t.Errorf("re-run %d must be Unchanged with nothing spliced:\n%s", run, out) + } + } +} + From bcdeec2253ccf6006445e74b4528f21cfbc3e8b8 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:07:58 +0000 Subject: [PATCH 11/13] fix(splice): refuse a loop-body change in any run, not only a run of one (#942) The sameLoopShell refusal was asked only when one declared statement met one stored one. A loop-body change beside another real change (a commit gaining `without events`) made a longer run, which was replaced: the loop rebuilt with new $IDs under mdl 1, where the same change alone is refused. Co-Authored-By: Claude Opus 5.5 --- mdl/executor/cmd_flow_modify.go | 38 +++++++++++++------ .../flow_commit_events_rerun_test.go | 38 +++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index afca1bd05b..25556d3961 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -839,17 +839,7 @@ func (pd *patchDiff) gap(ins []ast.MicroflowStatement, stored []ast.MicroflowSta return pd.statements(d.ElseBody, s.ElseBody) } if sameLoopShell(ins[0], del[0]) { - // The splice does not edit inside a loop, and the engine would - // take this as a replace of the whole loop: every node in it - // rebuilt, renumbered and redrawn — the rebuild's loss, confined - // to the loop but no less silent. An explicit `alter … replace - // loop` states that loss, so the refusal names it. - why := ¬Spliceable{reason: fmt.Sprintf("the %s changes inside its body; the splice does not edit inside a loop, "+ - "and replacing the whole loop would rebuild every node it holds", describeAt(del[0]))} - if c, err := pd.loc.locate(del[0]); err == nil { - why.loopHandle = c.Statement - } - return why + return pd.loopBodyChanged(del[0]) } if sameIgnoringLayout(ins[0], del[0]) { return redrawn(del[0]) @@ -867,6 +857,18 @@ func (pd *patchDiff) gap(ins []ast.MicroflowStatement, stored []ast.MicroflowSta } } } + // A stored loop declared again with a new body, in a run with other + // changes: replacing the run would rebuild the loop just as replacing it + // alone would, so it is refused the same way. Asked only of the 1:1 run, + // the refusal depended on what happened to change NEXT to the loop + // (ako/mxcli#942). + for _, d := range ins { + for _, st := range del { + if sameLoopShell(d, st) { + return pd.loopBodyChanged(st) + } + } + } // A stored `if` declared again with its condition but neither as the same // shell nor with only its condition changed has its else or a branch's // return added or taken away: where its paths end or meet changes, which @@ -1056,6 +1058,20 @@ func sameLoopShell(declared, stored ast.MicroflowStatement) bool { return false } +// loopBodyChanged refuses a change inside the body of the stored loop. The +// splice does not edit inside a loop, and the engine would take it as a replace +// of the whole loop: every node in it rebuilt, renumbered and redrawn — the +// rebuild's loss, confined to the loop but no less silent. An explicit +// `alter … replace loop` states that loss, so the refusal names it. +func (pd *patchDiff) loopBodyChanged(stored ast.MicroflowStatement) error { + why := ¬Spliceable{reason: fmt.Sprintf("the %s changes inside its body; the splice does not edit inside a loop, "+ + "and replacing the whole loop would rebuild every node it holds", describeAt(stored))} + if c, err := pd.loc.locate(stored); err == nil { + why.loopHandle = c.Statement + } + return why +} + // describeAt names a stored statement and where it is drawn, for a message. func describeAt(st ast.MicroflowStatement) string { if p := statementAnnotations(st); p != nil && p.Position != nil { diff --git a/mdl/roundtrip/flow_commit_events_rerun_test.go b/mdl/roundtrip/flow_commit_events_rerun_test.go index 377513bc8b..bace5b9857 100644 --- a/mdl/roundtrip/flow_commit_events_rerun_test.go +++ b/mdl/roundtrip/flow_commit_events_rerun_test.go @@ -114,3 +114,41 @@ func assertUnchangedRerun(t *testing.T, h *harness, script string) { } } +// The refusal must not depend on what changes NEXT to the loop. Once a +// spelling no longer passes for a change (above), a genuine change beside a +// loop-body change still made the diff run the two together, and a run of +// more than one statement was replaced without asking whether a loop in it +// was the stored loop with a new body: under mdl 1 the loop was rebuilt with +// new $IDs. The control is the loop-body change alone, which was always +// refused. +func TestSpliceRerun_LoopBodyChangeBesideAnotherChangeIsRefused(t *testing.T) { + h := newHarness(t) + defer h.close() + const flow = "Repro_LoopBeside" + for _, c := range []struct{ name, commit string }{ + {"commit changed beside the loop", "commit $L without events"}, + {"control: the loop-body change alone", "commit $L"}, + } { + t.Run(c.name, func(t *testing.T) { + h.restore() + if err := h.exec("mdl 1;\n" + commitEventsFlow(flow, "commit $L", "a")); err != nil { + t.Fatalf("create: %v\n%s", err, h.out.String()) + } + stored := drawnObjects(t, h.flowUnit(t, flow)) + before := h.snapshot() + err := h.exec("mdl 1;\n" + commitEventsFlow(flow, c.commit, "z")) + if err == nil || !strings.Contains(err.Error(), "changes inside its body") { + t.Errorf("want the refusal naming the loop, got %v\n%s", err, h.out.String()) + } + if changed := before.diff(h.snapshot()); len(changed) != 0 { + t.Errorf("the refused change wrote: %v", changed) + } + after := drawnObjects(t, h.flowUnit(t, flow)) + for id := range stored { + if _, ok := after[id]; !ok { + t.Errorf("stored object %s was renumbered away", id) + } + } + }) + } +} From 882543b420a411e28532f2880ded991eaf60b2fa Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:07:59 +0000 Subject: [PATCH 12/13] fix(describe): judge a loop body's merges against the microflow's flows (#942) A loop's object collection holds no flows; Mendix stores them all in the microflow's collection. droppedMergeWarnings recursed with the loop's own collection, so every in-loop merge had in-degree 0 and the merge closing an `if` at the end of a loop body was reported as one describe -> exec deletes. Co-Authored-By: Claude Opus 5.5 --- .../cmd_microflows_show_loop_merge_test.go | 76 +++++++++++++++++++ mdl/executor/cmd_microflows_show_merge.go | 37 ++++++++- 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 mdl/executor/cmd_microflows_show_loop_merge_test.go diff --git a/mdl/executor/cmd_microflows_show_loop_merge_test.go b/mdl/executor/cmd_microflows_show_loop_merge_test.go new file mode 100644 index 0000000000..b822fae072 --- /dev/null +++ b/mdl/executor/cmd_microflows_show_loop_merge_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// ako/mxcli#942: a loop's object collection holds the objects of its body but +// none of its flows — Mendix stores every flow of a microflow, the loop body's +// too, in the microflow's own collection. Judged against the loop's collection +// alone, every merge in a loop body had in-degree 0, so the `if` at the end of +// a loop body drew "the merge … joins no decision … re-executing this MDL +// DELETES it" on every describe, a freshly built flow included. +func loopBodyMergeFixture(t *testing.T, orphan bool) *microflows.MicroflowObjectCollection { + t.Helper() + obj := func(x int) microflows.BaseMicroflowObject { + return microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(randomTestID())}, Position: model.Point{X: x, Y: 130}} + } + inner := µflows.MicroflowObjectCollection{} + split := µflows.ExclusiveSplit{BaseMicroflowObject: obj(300)} + change := µflows.ActionActivity{BaseActivity: microflows.BaseActivity{BaseMicroflowObject: obj(400)}} + merge := µflows.ExclusiveMerge{BaseMicroflowObject: obj(500)} + inner.Objects = append(inner.Objects, split, change, merge) + + root := µflows.MicroflowObjectCollection{} + start := µflows.StartEvent{BaseMicroflowObject: obj(100)} + loop := µflows.LoopedActivity{BaseMicroflowObject: obj(200), ObjectCollection: inner} + end := µflows.EndEvent{BaseMicroflowObject: obj(700)} + root.Objects = append(root.Objects, start, loop, end) + edge := func(from, to microflows.MicroflowObject, value string) { + f := µflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID(randomTestID())}, + OriginID: from.GetID(), DestinationID: to.GetID(), + } + if value != "" { + f.CaseValue = µflows.ExpressionCase{Expression: value} + } + // Every flow in the microflow's own collection, as Mendix stores it. + root.Flows = append(root.Flows, f) + } + edge(start, loop, "") + edge(loop, end, "") + if orphan { + // Control: the merge has one incoming path and joins nothing. + edge(split, change, "true") + edge(change, merge, "") + other := µflows.EndEvent{BaseMicroflowObject: obj(450)} + inner.Objects = append(inner.Objects, other) + edge(split, other, "false") + } else { + // `if … then change …; end if;` as the last statement of the body: + // the true branch through the change, the false branch straight to + // the merge, which ends the body. + edge(split, change, "true") + edge(change, merge, "") + edge(split, merge, "false") + } + return root +} + +func TestDroppedMergeWarnings_LoopBodyMergeJudgedAgainstTheMicroflowsFlows(t *testing.T) { + root := loopBodyMergeFixture(t, false) + if got := droppedMergeWarnings(nil, root, labelRejoinMerges(root)); len(got) != 0 { + t.Errorf("flagged the merge closing an `if` at the end of a loop body: %v", got) + } + // Control: a genuinely orphan merge in a loop body still warns. + root = loopBodyMergeFixture(t, true) + if got := droppedMergeWarnings(nil, root, labelRejoinMerges(root)); len(got) != 1 { + t.Errorf("an orphan merge in a loop body: got %d warnings, want 1: %v", len(got), got) + } +} diff --git a/mdl/executor/cmd_microflows_show_merge.go b/mdl/executor/cmd_microflows_show_merge.go index 4ac083a010..b21194094d 100644 --- a/mdl/executor/cmd_microflows_show_merge.go +++ b/mdl/executor/cmd_microflows_show_merge.go @@ -264,8 +264,31 @@ func mergeDeclarationLines(indent int, label string, obj microflows.MicroflowObj // // Loop bodies are recursed into because a LoopedActivity owns its own object // collection and its own traversal; without that every in-loop if/else merge -// would report as dropped. +// would report as dropped. A loop's collection holds the body's OBJECTS but not +// its flows: Mendix stores every flow of the microflow in the microflow's own +// collection. So each body is judged against the flows of the whole microflow +// — taken against its own collection alone, every merge in a loop body had +// in-degree 0 and was reported dropped (ako/mxcli#942). func droppedMergeWarnings(ctx *ExecContext, oc *microflows.MicroflowObjectCollection, labels mergeLabels) []string { + return droppedMergeWarningsIn(ctx, oc, allFlows(oc), labels) +} + +// allFlows is every sequence flow of a microflow: its own collection's and, +// should a loop's collection hold any, those too. +func allFlows(oc *microflows.MicroflowObjectCollection) []*microflows.SequenceFlow { + if oc == nil { + return nil + } + flows := append([]*microflows.SequenceFlow(nil), oc.Flows...) + for _, o := range oc.Objects { + if loop, ok := o.(*microflows.LoopedActivity); ok { + flows = append(flows, allFlows(loop.ObjectCollection)...) + } + } + return flows +} + +func droppedMergeWarningsIn(ctx *ExecContext, oc *microflows.MicroflowObjectCollection, flows []*microflows.SequenceFlow, labels mergeLabels) []string { if oc == nil { return nil } @@ -279,7 +302,13 @@ func droppedMergeWarnings(ctx *ExecContext, oc *microflows.MicroflowObjectCollec // Merges that a split joins on are spelled by `end if` / `end split`. represented := map[model.ID]bool{} - for _, mergeID := range findSplitMergePoints(ctx, oc, activityMap) { + flowsByOrigin := make(map[model.ID][]*microflows.SequenceFlow) + for _, f := range flows { + if f != nil { + flowsByOrigin[f.OriginID] = append(flowsByOrigin[f.OriginID], f) + } + } + for _, mergeID := range findSplitMergePointsForGraph(ctx, activityMap, flowsByOrigin) { represented[mergeID] = true } @@ -287,7 +316,7 @@ func droppedMergeWarnings(ctx *ExecContext, oc *microflows.MicroflowObjectCollec // as the continuation of whatever construct closes there — including the // split-with-a-returning-branch that findSplitMergePoints cannot pair up. inDegree := map[model.ID]int{} - for _, f := range oc.Flows { + for _, f := range flows { if f != nil { inDegree[f.DestinationID]++ } @@ -318,7 +347,7 @@ func droppedMergeWarnings(ctx *ExecContext, oc *microflows.MicroflowObjectCollec // against its own collection. for _, o := range oc.Objects { if loop, ok := o.(*microflows.LoopedActivity); ok { - out = append(out, droppedMergeWarnings(ctx, loop.ObjectCollection, labels)...) + out = append(out, droppedMergeWarningsIn(ctx, loop.ObjectCollection, flows, labels)...) } } return out From 22c866e0cbec3f7b0338d3ec6d67b5f20901770d Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 12:07:59 +0000 Subject: [PATCH 13/13] docs: changelog, findings and bug-pattern note for #942 Co-Authored-By: Claude Opus 5.5 --- .claude/skills/fix-issue/findings/mdl-executor.jsonl | 3 +++ CHANGELOG.md | 1 + docs-wiki/bug-patterns/scripts-that-cannot-rerun.md | 12 ++++++++++++ 3 files changed, 16 insertions(+) diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index c672869457..4840c09035 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -836,3 +836,6 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: TestApp WorkflowCommons snippets break exec — `combobox … (Attribute: TimeFrame)` \"has no entity to bind against\", image `Visible: CompletionType in (…)` \"place the widget inside a data container\"; widgets sit directly in a snippet with a parameter, no data view", "cause": "Studio Pro binds a widget outside every data container to a page/snippet PARAMETER: AttributeRef (or ConditionalVisibilitySettings.Attribute, or a combo box IndirectEntityRef) beside SourceVariable Forms$PageVariable {SnippetParameter|PageParameter: name, Widget: \"\"}. describe printed the attribute bare and exec had no spelling for the source, so it refused (or, before describe/pluggable Visible were fixed, silently wrote no binding)", "file": "`mdl/executor/cmd_pages_parameter_binding.go`, `cmd_pages_builder_v3.go` (resolveInputBinding/parameterVariable), `widget_engine.go` (Attribute/Association mappings), `cmd_pages_builder_visible_when.go`, describe in `cmd_pages_describe_parse.go`/`cmd_pages_describe_pluggable.go`, writer `widgetobj.SetSourceVariable`, `conditionalVisibilityToGen`", "insight": "A newly fixed describe gap can surface as an exec refusal the allowlist never expected: the refusal was right for the bare spelling and the cure is a spelling for the stored source (`$Param.Attr`, `$Param.Module.Assoc`, `Visible: $Param.Attr in (…)`), not a looser guard. Survey SourceVariable slot combinations (W/P/S/L) across the fixture first: TestApp had --S- on built-in inputs, pluggable values and 76 visibility settings, all unspelled", "refs": ["#721", "#826"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "describe prints a pluggable image's `Visible: Attr in (…)` (and expression Visible/Editable) twice", "cause": "the image branch appended appendConditionalProps and then appendAppearanceProps, which appends the same conditional settings", "file": "`mdl/executor/cmd_pages_describe_output.go` (image branch)", "insight": "appendAppearanceProps already owns visibility/editability; a branch that also appends them duplicates the key — grep for both on one widget kind", "refs": ["#721"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "integration roundtrip: describe → exec of TestApp's WorkflowCommons.UserTask_Assign fails: \"widget `grid8` (datagrid) cannot have its visibility set: its widget package declares no Visibility system property\"", "cause": "MDL-WIDGET41 gated Visible: on the package declaring , measured only from which packages declare it — but Studio Pro stores ConditionalVisibilitySettings on a Datagrid whose Type declares no Visibility property, and mxbuild 11.14 accepts static and conditional visibility there (0 errors)", "file": "`mdl/executor/pluggable_system_props.go` (`undeclaredSystemProps`)", "insight": "A declared system property is evidence of where a setting is shown, not of whether it exists; a refusal must be measured against what Studio Pro actually stores. Run the roundtrip harness over Studio Pro-authored pages before adding a refusal on stored shapes. Editability stays gated", "refs": []} +{"area": "mdl/executor", "date": "2026-10-03", "symptom": "create or modify of a flow with `commit $L with events`: an unchanged re-exec reports \"Unchanged microflow … (spliced: 1 replaced)\"; a loop-body change next to it is not refused under mdl 1 but rebuilds the loop with new $IDs (\"spliced: 1 replaced, 1 dropped\")", "cause": "matchValue compared MfCommitStmt.ExplicitWithEvents, a spelling-only flag (MDL067), and describe prints the stored commit bare; InheritanceSplitStmt.LegacyCaseKeyword/LegacyElseKeyword and an empty `else` (IfStmt.HasElse) had the same defect", "file": "`mdl/executor/flow_declared_match.go` (`spellingFields`), `mdl/executor/flow_canonical.go` (`canonicalNested`)", "insight": "Any AST field that records how something was written rather than what is stored must be named in spellingFields or canonicalised, or the statement never matches its own description. The unchanged re-run only shows it where the built comparison does not decide: make the stored flow differ from the builder's drawing (a moved node) before re-running", "refs": ["ako/mxcli#942"]} +{"area": "mdl/executor", "date": "2026-10-03", "symptom": "under mdl 1, a loop-body change next to a genuinely changed statement (e.g. commit gains `without events`) is spliced as \"1 replaced, 1 dropped\" and rebuilds the loop with new $IDs; the same loop-body change alone is refused", "cause": "the sameLoopShell refusal was asked only of a 1:1 run of unmatched statements; a longer run went straight to replace", "file": "`mdl/executor/cmd_flow_modify.go` (`patchDiff.gap`, `loopBodyChanged`)", "insight": "A refusal that guards identity must be asked of every pairing in the run, not of the run's shape; what happens to change next to the guarded node decides the shape", "refs": ["ako/mxcli#942"]} +{"area": "mdl/executor", "date": "2026-10-03", "symptom": "describe warns \"the merge at (x, y) … joins no decision … re-executing this MDL DELETES it\" for an `if` at the end of a loop body, even on a freshly built flow", "cause": "droppedMergeWarnings recursed into a loop with the loop's own collection, which holds the body's objects but none of its flows (Mendix stores every flow in the microflow's collection), so every in-loop merge had in-degree 0", "file": "`mdl/executor/cmd_microflows_show_merge.go` (`droppedMergeWarningsIn`, `allFlows`)", "insight": "A loop's ObjectCollection has no flows; anything judging a loop body's graph needs the root collection's flows. The existing unit fixture put flows in the loop collection, the shape Mendix never stores, so it passed", "refs": ["ako/mxcli#942"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index f9ac22a378..3e9de98485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`create or modify` of a flow matches `commit … with events`, a legacy `split type` spelling and an empty `else` against what is stored** (ako/mxcli#942). Describe prints a stored commit as a bare `commit`, the `when … then` split form, and no empty `else`. The statement diff compared the spelling, so these never matched their own activity. An unchanged re-run reported "Unchanged … (spliced: 1 replaced)". A change inside a loop body next to such a statement was not refused under `mdl 1`, and the loop was rebuilt with new element IDs. `without events` is still a change. **The loop-body refusal also holds when another statement changes next to the loop:** before, only a loop-body change on its own was refused. **`describe` no longer warns that the merge closing an `if` at the end of a loop body "joins no decision"** and would be deleted. The check counted a loop body's flows from the loop's own collection, which holds none. - **`page.title` and `catalog.pages.Title` are the project's default language** (mendixlabs/mxcli#1262) — the catalog took whichever translation of a page title a Go map range met first, so a lint rule reading `page.title` changed output between catalog rebuilds of an unchanged project. The title is now the project's default language, else en_US, else the lowest-sorted non-empty language; every translation stays available in `catalog.strings` (`StringContext = 'Forms$Page.Title'`). The catalog schema version moves to 17, so a cached catalog is rebuilt. - **Lint rules that read full-catalog data no longer pass silently** — `widgets()`, `xpath_expressions()`, `activities_for()`, `permissions()`, `permissions_for()` and a page's or snippet's `widget_count` read data only `refresh catalog full` writes, but only `refs_to` / `refs_from` raised the build depth, so unless another rule in the set happened to call `refs_to` the build stayed fast, they returned `[]` / `0` and the rule reported nothing (`widgets()`: 0 rows vs 46 on PedApp). They are now auto-detected like `refs_to`. The built-in MPR005, MPR006 and MPR012 read widgets the same way and were silent on a project without `.claude/lint-rules/` (an empty container went unreported); they now request the full build, so `mxcli lint` always builds a full catalog (measured on TestApp: 2.3 s -> 3.1 s for the build). diff --git a/docs-wiki/bug-patterns/scripts-that-cannot-rerun.md b/docs-wiki/bug-patterns/scripts-that-cannot-rerun.md index c1a02b282c..a87ef6ff8d 100644 --- a/docs-wiki/bug-patterns/scripts-that-cannot-rerun.md +++ b/docs-wiki/bug-patterns/scripts-that-cannot-rerun.md @@ -103,6 +103,18 @@ every run until the splice could replace an activity's notes. The header-only upgrade (a mdl 0 description under `mdl 1;`) is where both surface on Studio Pro content, because a backslash escape in a note is a real change under mdl 1. +The same failure has a structural cousin: an AST field that records how a +statement was **spelled**, kept for a deprecation or lint note, is compared like +a stored property unless it is named as spelling (`spellingFields`; an empty +`else` goes through `canonicalFlow` instead, because the `if` shell compare +needs `HasElse`). `commit … with events` never matched its stored bare commit +(#942). It is not cosmetic. The diff groups adjacent unmatched statements into +one run, and the loop-body refusal was asked only of a run of one. So a +spelling difference next to a loop turned a refused loop-body edit into a silent +replace of the loop, with every `$ID` in it re-minted. When you add an AST flag, +ask whether describe can print it. Refusals have to hold for any run that pairs +a stored loop with a new body, whatever else changes next to it. + **Some re-runs only reach a no-op across two statements.** `revoke all` followed by the grant that should hold ends with the rules it started with, but each statement is a real write of its own. The grant's write was compared with a unit