From 9ed0b279a0d0f7c39d02c6bfa4534afb10275d65 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:18:44 +0000 Subject: [PATCH 01/21] fix: end a multi-file check on a verdict for the whole set (mendixlabs/mxcli#1431) Reported: "`mxcli check -p R/R.mpr bad.mdl good.mdl` ... the final line is 'Check passed!' even though the exit code is 1"; reversing the file order flips the last line, so anything reading the log tail sees success. runCheckFiles kept only the worst exit code; each file printed its own verdict and nothing printed one for the set. It now closes with "N of M files failed: a.mdl, b.mdl" or "All M files passed". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...es-ends-on-check-passed-after-failure.json | 1 + cmd/mxcli/check_multi_file_summary_test.go | 44 +++++++++++++++++++ cmd/mxcli/cmd_check.go | 14 +++++- 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json create mode 100644 cmd/mxcli/check_multi_file_summary_test.go diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json new file mode 100644 index 000000000..dc40fb893 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-check-several-files-ends-on-check-passed-after-failure.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli check -p app.mpr bad.mdl good.mdl` exits 1 but its last line is \"Check passed!\"; reversing the file order flips the last line, so anything reading the log tail sees success", "cause": "`runCheckFiles` ran `runCheckFileAgainst` per file and kept only the worst exit code; each file prints its own verdict and nothing printed one for the set, so the last file's \"Check passed!\" (plus its no-project note) closed the run", "file": "`cmd/mxcli/cmd_check.go` (`runCheckFiles`)", "insight": "A per-unit verdict line is read as the run's verdict by whoever tails the log (agents especially); when a command loops over inputs, the loop owner must print the aggregate last. The per-file \"Check passed!\" is followed by the two-line no-project note, so assert on the true last line rather than `Contains`, or the test passes against the bug. No project needed for the test: a syntax error makes the failing file", "refs": ["mendixlabs/mxcli#1431"]} diff --git a/cmd/mxcli/check_multi_file_summary_test.go b/cmd/mxcli/check_multi_file_summary_test.go new file mode 100644 index 000000000..7904a4e83 --- /dev/null +++ b/cmd/mxcli/check_multi_file_summary_test.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "strings" + "testing" +) + +// A check of several files ends on a verdict for the whole set +// (mendixlabs/mxcli#1431). It used to end on the last file's own +// "Check passed!", so a failing file followed by a passing one read as a pass +// to anything looking at the tail of the log; only the exit code said 1. +func TestCheck_ScriptSetEndsOnOverallVerdict(t *testing.T) { + if f := checkCmd.Flags().Lookup("format"); f != nil { + _ = f.Value.Set(f.DefValue) + } + scripts := t.TempDir() + bad := writeScript(t, scripts, "bad.mdl", "create persistent entity M.E ( Name: String(100) \n") + good := writeScript(t, scripts, "good.mdl", "create persistent entity M.F ( Name: String(100) );\n") + + lastLine := func(out string) string { + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + return strings.TrimSpace(lines[len(lines)-1]) + } + + var code int + out := captureStd(t, func() { code = runCheckFiles(checkCmd, []string{bad, good}) }) + if code == 0 { + t.Fatalf("a set with a failing file exited 0:\n%s", out) + } + if want := "1 of 2 files failed: " + bad; lastLine(out) != want { + t.Errorf("last line = %q, want %q\n%s", lastLine(out), want, out) + } + + // Control: a set that passes says so for the whole set. + out = captureStd(t, func() { code = runCheckFiles(checkCmd, []string{good, good}) }) + if code != 0 { + t.Fatalf("a passing set exited %d:\n%s", code, out) + } + if want := "All 2 files passed"; lastLine(out) != want { + t.Errorf("last line = %q, want %q\n%s", lastLine(out), want, out) + } +} diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index e1d45cf44..f161746b8 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -577,7 +577,7 @@ func runCheckFiles(cmd *cobra.Command, files []string) int { scratch = sc } worst := 0 - var notApplied []string + var notApplied, failed []string for i, f := range files { if i > 0 { fmt.Println() @@ -596,12 +596,24 @@ func runCheckFiles(cmd *cobra.Command, files []string) int { if code > worst { worst = code } + if code != 0 { + failed = append(failed, f) + } if scratch != nil && i < len(files)-1 { if why := applyToScratch(scratch, f, code); why != "" { notApplied = append(notApplied, why) } } } + // Each file ends on its own verdict, so without this the run's last line + // was the last file's "Check passed!" even after an earlier file failed + // (mendixlabs/mxcli#1431). Close on the verdict for the whole set. + fmt.Println() + if len(failed) > 0 { + fmt.Printf("%d of %d files failed: %s\n", len(failed), len(files), strings.Join(failed, ", ")) + } else { + fmt.Printf("All %d files passed\n", len(files)) + } return worst } From 8f79f4ea44aaa919bb3a7a44c132bd8b9daae9fc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:24:19 +0000 Subject: [PATCH 02/21] fix: marketplace download --output accepts a directory (mendixlabs/mxcli#1443) Reported: "`mxcli marketplace download --output ` ... fails with a rename error reading `rename ... file exists`". The temp file was created in filepath.Dir(output) and renamed onto output itself. An --output that is an existing directory, or ends in a separator, now gets the temp file inside it and the package written as / (or the content--.mpk fallback). Flag help says "file or directory". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...d-output-directory-rename-file-exists.json | 1 + cmd/mxcli/cmd_marketplace.go | 30 +++- cmd/mxcli/cmd_marketplace_download_test.go | 152 +++++++----------- 3 files changed, 82 insertions(+), 101 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json new file mode 100644 index 000000000..a1b13ba58 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-marketplace-download-output-directory-rename-file-exists.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli marketplace download --output ` fails with `write : rename /.mxcli-download-*.part : file exists`; only a file path works", "cause": "The command streams to a temp file in `filepath.Dir(output)` and renames it onto `output`, treating `--output` as a file path unconditionally; rename(2) onto a non-empty directory is EEXIST", "file": "`cmd/mxcli/cmd_marketplace.go` (`runMarketplaceDownload`)", "insight": "The temp-then-rename pattern decides the destination before it knows what `--output` names. Resolve directory-ness first (existing dir, or trailing separator = dir to create), put the temp file inside it so the rename stays on one filesystem, and join the CDN filename (or the content--.mpk fallback). Testable offline: `runMarketplace` + `marketplaceClientFactory` with one httptest server serving versions, the 303 hop and the CDN file — the CDN hop uses http.DefaultClient, which reaches a plain-http httptest server fine", "refs": ["mendixlabs/mxcli#1443"]} diff --git a/cmd/mxcli/cmd_marketplace.go b/cmd/mxcli/cmd_marketplace.go index f6f5c3bf3..3a9cb256c 100644 --- a/cmd/mxcli/cmd_marketplace.go +++ b/cmd/mxcli/cmd_marketplace.go @@ -72,10 +72,12 @@ var marketplaceDownloadCmd = &cobra.Command{ By default the latest version is downloaded. Use --version to pick a specific version number. The file is written to the current directory under its CDN -filename unless -o is given.`, +filename unless -o is given; -o takes a file path, or a directory to write the +CDN filename into.`, Example: ` mxcli marketplace download 2888 mxcli marketplace download 2888 --version 7.0.2 - mxcli marketplace download 2888 -o ./mods/dbc.mpk`, + mxcli marketplace download 2888 -o ./mods/dbc.mpk + mxcli marketplace download 2888 -o ./mods/`, Args: cobra.ExactArgs(1), RunE: runMarketplaceDownload, } @@ -95,7 +97,7 @@ func init() { marketplaceDownloadCmd.Flags().String("profile", auth.ProfileDefault, "credential profile") marketplaceDownloadCmd.Flags().String("version", "", "version number to download (default: latest)") - marketplaceDownloadCmd.Flags().StringP("output", "o", "", "output path (default: CDN filename in the current directory)") + marketplaceDownloadCmd.Flags().StringP("output", "o", "", "output file or directory (default: CDN filename in the current directory)") marketplaceCmd.AddCommand(marketplaceSearchCmd) marketplaceCmd.AddCommand(marketplaceInfoCmd) @@ -150,9 +152,26 @@ func runMarketplaceDownload(cmd *cobra.Command, args []string) error { // Stream to a temp file first, then derive the final name and rename, so a // failed/cancelled download never leaves a truncated .mpk in place. + // + // An --output naming a directory (an existing one, or any path ending in a + // separator) receives the package under the name it would otherwise get; + // renaming the temp file onto the directory itself failed with "file + // exists" (mendixlabs/mxcli#1443). dir := "." + intoDir := false if output != "" { dir = filepath.Dir(output) + if strings.HasSuffix(output, "/") || strings.HasSuffix(output, string(os.PathSeparator)) { + intoDir = true + } else if fi, err := os.Stat(output); err == nil && fi.IsDir() { + intoDir = true + } + if intoDir { + if err := os.MkdirAll(output, 0o755); err != nil { + return fmt.Errorf("create output directory: %w", err) + } + dir = output + } } tmp, err := os.CreateTemp(dir, ".mxcli-download-*.part") if err != nil { @@ -171,11 +190,14 @@ func runMarketplaceDownload(cmd *cobra.Command, args []string) error { } dest := output - if dest == "" { + if dest == "" || intoDir { if filename == "" { filename = fmt.Sprintf("content-%d-%s.mpk", contentID, version.VersionNumber) } dest = filename + if intoDir { + dest = filepath.Join(output, filename) + } } if err := os.Rename(tmpName, dest); err != nil { return fmt.Errorf("write %s: %w", dest, err) diff --git a/cmd/mxcli/cmd_marketplace_download_test.go b/cmd/mxcli/cmd_marketplace_download_test.go index 1411bff0b..aedb0a201 100644 --- a/cmd/mxcli/cmd_marketplace_download_test.go +++ b/cmd/mxcli/cmd_marketplace_download_test.go @@ -3,128 +3,86 @@ package main import ( - "bytes" "fmt" "net/http" "os" "path/filepath" "strings" "testing" - - "github.com/mendixlabs/mxcli/internal/marketplace" ) -func TestStartSearchProgress_Gating(t *testing.T) { - t.Run("nil for --json", func(t *testing.T) { - c := marketplace.New(http.DefaultClient) - if startSearchProgress(c, os.Stderr, true) != nil { - t.Error("expected no progress for --json output") - } - if c.OnProgress != nil { - t.Error("OnProgress must stay unset for --json") - } - }) - t.Run("nil for non-terminal writer", func(t *testing.T) { - c := marketplace.New(http.DefaultClient) - // A bytes.Buffer is not an *os.File, so it's never an interactive terminal. - if startSearchProgress(c, &bytes.Buffer{}, false) != nil { - t.Error("expected no progress for a non-terminal writer (pipe/buffer)") - } - if c.OnProgress != nil { - t.Error("OnProgress must stay unset for a non-terminal writer") - } - }) -} - -func TestResolveVersion(t *testing.T) { - versions := []marketplace.Version{ - {VersionNumber: "7.0.3", DownloadURL: "u3"}, - {VersionNumber: "7.0.2", DownloadURL: "u2"}, - } - t.Run("latest (empty) picks first", func(t *testing.T) { - v, err := resolveVersion(versions, "") - if err != nil || v.VersionNumber != "7.0.3" { - t.Fatalf("got %v, %v; want 7.0.3", v, err) - } - }) - t.Run("specific match", func(t *testing.T) { - v, err := resolveVersion(versions, "7.0.2") - if err != nil || v.VersionNumber != "7.0.2" { - t.Fatalf("got %v, %v; want 7.0.2", v, err) - } - }) - t.Run("not found", func(t *testing.T) { - if _, err := resolveVersion(versions, "9.9.9"); err == nil { - t.Fatal("expected error for missing version") - } - }) - t.Run("empty list", func(t *testing.T) { - if _, err := resolveVersion(nil, ""); err == nil { - t.Fatal("expected error for empty version list") - } - }) -} - -// marketplaceTestHandler serves the content/versions/download/CDN endpoints for a -// single content item, building absolute URLs from the request host so the -// download redirect points back at the same test server. -func marketplaceTestHandler(contentType, filename, mpkBody string) http.HandlerFunc { +// downloadHandler serves a one-version list whose download resolves (303) to +// a CDN path ending in cdnName, the two hops Client.Download makes. +func downloadHandler(t *testing.T, cdnName, body string) http.HandlerFunc { + t.Helper() return func(w http.ResponseWriter, r *http.Request) { base := "http://" + r.Host switch { - case strings.HasSuffix(r.URL.Path, "/versions"): - fmt.Fprintf(w, `{"items":[{"name":"X","versionId":"v1","versionNumber":"1.0.0","minSupportedMendixVersion":"10.0.0","publicationDate":"2026-01-01T00:00:00Z","downloadUrl":%q}]}`, base+"/v1/versions/v1/download") - case strings.HasSuffix(r.URL.Path, "/download"): - http.Redirect(w, r, base+"/cdn/"+filename, http.StatusSeeOther) - case strings.HasPrefix(r.URL.Path, "/cdn/"): - _, _ = w.Write([]byte(mpkBody)) - case strings.HasPrefix(r.URL.Path, "/v1/content/"): - fmt.Fprintf(w, `{"contentId":42,"publisher":"Mendix","type":%q,"isPrivate":false}`, contentType) + case strings.HasPrefix(r.URL.Path, "/v1/content/170/versions"): + fmt.Fprintf(w, `{"items":[{"name":"Community Commons","versionNumber":"11.5.0","downloadUrl":%q}]}`, base+"/dl/170") + case r.URL.Path == "/dl/170": + http.Redirect(w, r, base+"/cdn/"+cdnName, http.StatusSeeOther) + case r.URL.Path == "/cdn/"+cdnName: + _, _ = w.Write([]byte(body)) default: - w.WriteHeader(http.StatusNotFound) + t.Errorf("unexpected request %s", r.URL) + http.NotFound(w, r) } } } -func TestMarketplaceDownload_WritesFile(t *testing.T) { - dest := filepath.Join(t.TempDir(), "out.mpk") - out, err := runMarketplace(t, marketplaceTestHandler("Module", "DatabaseConnector-v1.0.0.mpk", "PK\x03\x04mpk"), - "download", "42", "-o", dest) - if err != nil { - t.Fatalf("download failed: %v\n%s", err, out) - } - got, err := os.ReadFile(dest) +// `--output ` writes the package into that directory under its +// CDN filename (mendixlabs/mxcli#1443). It used to stream to a temp file +// beside the directory and rename it onto the directory: "rename … file exists". +func TestMarketplaceDownload_OutputDirectory(t *testing.T) { + dir := t.TempDir() + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "--output", dir) if err != nil { - t.Fatalf("output not written: %v", err) + t.Fatalf("download into an existing directory: %v\n%s", err, out) } - if string(got) != "PK\x03\x04mpk" { - t.Errorf("content = %q, want the mpk body", string(got)) + want := filepath.Join(dir, "CommunityCommons.mpk") + got, rerr := os.ReadFile(want) + if rerr != nil || string(got) != "mpk-bytes" { + t.Fatalf("want the package at %s, read %q (%v)\n%s", want, got, rerr, out) } - // No temp .part file left behind in the dest directory. - entries, _ := os.ReadDir(filepath.Dir(dest)) - for _, e := range entries { - if strings.HasSuffix(e.Name(), ".part") { - t.Errorf("leftover temp file: %s", e.Name()) - } + if !strings.Contains(out, want) { + t.Errorf("output does not name %s:\n%s", want, out) } + assertNoPartFiles(t, dir) + assertNoPartFiles(t, filepath.Dir(dir)) } -func TestMarketplaceInstall_WidgetRouting(t *testing.T) { - projDir := t.TempDir() - mpr := filepath.Join(projDir, "app.mpr") - if err := os.WriteFile(mpr, []byte("x"), 0o644); err != nil { - t.Fatal(err) - } - out, err := runMarketplace(t, marketplaceTestHandler("Widget", "RadioButtonList.mpk", "PK\x03\x04widget"), - "install", "42", "-p", mpr) +// A trailing separator names a directory even when it does not exist yet. +func TestMarketplaceDownload_OutputTrailingSeparator(t *testing.T) { + dir := filepath.Join(t.TempDir(), "mods") + string(os.PathSeparator) + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "-o", dir) if err != nil { - t.Fatalf("install failed: %v\n%s", err, out) + t.Fatalf("download into a new directory: %v\n%s", err, out) } - got, err := os.ReadFile(filepath.Join(projDir, "widgets", "RadioButtonList.mpk")) + if _, err := os.Stat(filepath.Join(dir, "CommunityCommons.mpk")); err != nil { + t.Fatalf("package not written into %s: %v\n%s", dir, err, out) + } +} + +// Control: a file path keeps working as before. +func TestMarketplaceDownload_OutputFile(t *testing.T) { + dest := filepath.Join(t.TempDir(), "dbc.mpk") + out, err := runMarketplace(t, downloadHandler(t, "CommunityCommons.mpk", "mpk-bytes"), + "download", "170", "-o", dest) if err != nil { - t.Fatalf("widget not placed in widgets/: %v\noutput: %s", err, out) + t.Fatalf("download to a file path: %v\n%s", err, out) } - if string(got) != "PK\x03\x04widget" { - t.Errorf("widget content = %q", string(got)) + if got, err := os.ReadFile(dest); err != nil || string(got) != "mpk-bytes" { + t.Fatalf("want the package at %s, read %q (%v)", dest, got, err) + } +} + +func assertNoPartFiles(t *testing.T, dir string) { + t.Helper() + parts, _ := filepath.Glob(filepath.Join(dir, ".mxcli-download-*.part")) + if len(parts) > 0 { + t.Errorf("temp files left behind in %s: %v", dir, parts) } } From 9fc3cabfd9e3715183872e9046399f3bdeddd82f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:29:57 +0000 Subject: [PATCH 03/21] fix: wire rules into the describe subcommand (mendixlabs/mxcli#1402) Reported: `mxcli describe rule PgVectorKnowledgeBase.Rule_...` answers "Unknown type: rule", although `show rules` lists 14 and the MDL statement `describe rule ...` is handled by the executor. The describe subcommand keeps its own copy of the DESCRIBE type list (describeMDLCommand) and two auto-detect maps; none of them knew rules, so the explicit form was refused and the bare `describe Mod.Rule` form found "no describable document". Add RULE -> DESCRIBE RULE, catalog ObjectType RULE and unit type Microflows$Rule -> rule, and list rule in the help and the Valid types line. The "which is not a rule in this project" refusal from the same report is a downstream effect and is left alone. Test: TestDescribeMDLCommand_Rule (cmd/mxcli). Control: with the fix reverted it fails with "describe rule M.R: Unknown type: rule". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...0-12-describe-subcommand-rejects-rule.json | 1 + cmd/mxcli/cmd_describe.go | 10 ++++--- cmd/mxcli/cmd_describe_test.go | 27 +++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json new file mode 100644 index 000000000..a2cefa946 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-subcommand-rejects-rule.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli describe rule M.Rule_X` prints \"Unknown type: rule\" and bare `mxcli describe M.Rule_X` says no describable document is found, while `mxcli -c \"describe rule M.Rule_X\"` works", "cause": "Same drift as #1347: describeMDLCommand had no RULE case, and the auto-detect maps (catalog ObjectType RULE, unit $Type Microflows$Rule) omitted rules — the unit map even said so on purpose, a leftover from before the executor could describe rules", "file": "cmd/mxcli/cmd_describe.go", "fix": "Added RULE -> DESCRIBE RULE, objectTypeToDescribe[RULE] and unitTypeToDescribe[Microflows$Rule] = rule, plus the help and Valid-types lines", "insight": "Second instance of the subcommand's hand-kept type list lagging the DESCRIBE grammar. The executor's own bare-DESCRIBE map (mdl/executor/describe_auto.go objectTypeToDescribeKind) already had RULE — diffing that map's keys against cmd/mxcli's objectTypeToDescribe is the quickest way to find the next missing doctype. The 'not a rule in this project' refusal in the same report is a downstream effect, not a describe bug.", "refs": ["mendixlabs/mxcli#1402"]} diff --git a/cmd/mxcli/cmd_describe.go b/cmd/mxcli/cmd_describe.go index 70cde93b9..330cbca64 100644 --- a/cmd/mxcli/cmd_describe.go +++ b/cmd/mxcli/cmd_describe.go @@ -39,6 +39,7 @@ Types: constant Describe a constant microflow Describe a microflow nanoflow Describe a nanoflow + rule Describe a rule workflow Describe a workflow page Describe a page snippet Describe a snippet @@ -133,7 +134,7 @@ Example: mdlCmd, ok := describeMDLCommand(objectType, name) if !ok { fmt.Fprintf(os.Stderr, "Unknown type: %s\n", strings.Join(args[:len(args)-1], " ")) - fmt.Fprintln(os.Stderr, "Valid types: module, entity, association, enumeration, constant, microflow, nanoflow, workflow, page, snippet, layout, javaaction, javascriptaction, jsonstructure, importmapping, exportmapping, consumedrestservice, consumedodataservice, publishedodataservice, publishedrestservice, imagecollection, menu, taskqueue, scheduledevent, businesseventservice, databaseconnection, agent, aimodel, knowledgebase, consumedmcpservice, datatransformer, modulerole, userrole, appsecurity, settings, demouser, navigation, systemoverview") + fmt.Fprintln(os.Stderr, "Valid types: module, entity, association, enumeration, constant, microflow, nanoflow, rule, workflow, page, snippet, layout, javaaction, javascriptaction, jsonstructure, importmapping, exportmapping, consumedrestservice, consumedodataservice, publishedodataservice, publishedrestservice, imagecollection, menu, taskqueue, scheduledevent, businesseventservice, databaseconnection, agent, aimodel, knowledgebase, consumedmcpservice, datatransformer, modulerole, userrole, appsecurity, settings, demouser, navigation, systemoverview") fmt.Fprintln(os.Stderr, "Multi-word types also accepted: json structure, import mapping, export mapping, consumed rest service, consumed odata service, published odata service, published rest service, task queue, app security, image collection, scheduled event, regular expression, business event service, ai model, knowledge base, consumed mcp service, data transformer, etc.") os.Exit(1) } @@ -257,6 +258,8 @@ func describeMDLCommand(objectType, name string) (mdlCmd string, ok bool) { return fmt.Sprintf("DESCRIBE MICROFLOW %s", name), true case "NANOFLOW": return fmt.Sprintf("DESCRIBE NANOFLOW %s", name), true + case "RULE": + return fmt.Sprintf("DESCRIBE RULE %s", name), true case "WORKFLOW": return fmt.Sprintf("DESCRIBE WORKFLOW %s", name), true case "PAGE": @@ -348,6 +351,7 @@ var objectTypeToDescribe = map[string]string{ "ASSOCIATION": "association", "MICROFLOW": "microflow", "NANOFLOW": "nanoflow", + "RULE": "rule", "PAGE": "page", "SNIPPET": "snippet", "LAYOUT": "layout", @@ -382,11 +386,11 @@ var objectTypeToDescribe = map[string]string{ // unitTypeToDescribe maps a top-level unit's BSON $Type to the `describe` // keyword. Used by the live-reader fallback. Only directly describable document -// types are listed (page templates, building blocks, rules, etc. are absent so -// they don't resolve). +// types are listed (page templates etc. are absent so they don't resolve). var unitTypeToDescribe = map[string]string{ "Microflows$Microflow": "microflow", "Microflows$Nanoflow": "nanoflow", + "Microflows$Rule": "rule", "Forms$Page": "page", "Forms$Snippet": "snippet", "Pages$BuildingBlock": "buildingblock", diff --git a/cmd/mxcli/cmd_describe_test.go b/cmd/mxcli/cmd_describe_test.go index 67131716a..b5231c3ff 100644 --- a/cmd/mxcli/cmd_describe_test.go +++ b/cmd/mxcli/cmd_describe_test.go @@ -187,3 +187,30 @@ func TestDescribeHelp_StudioProDocumentNames(t *testing.T) { t.Errorf("unitTypeToDescribe[Queues$Queue] = %q, want taskqueue", got) } } + +// TestDescribeMDLCommand_Rule is mendixlabs/mxcli#1402: `describe rule M.R` +// answered "Unknown type: rule" although `-c "describe rule M.R"` works, and a +// bare `describe M.R` found "no describable document" because neither +// auto-detect map knew rules. +func TestDescribeMDLCommand_Rule(t *testing.T) { + const want = "DESCRIBE RULE M.R" + got, ok := describeMDLCommand("RULE", "M.R") + if !ok { + t.Fatal("describe rule M.R: Unknown type: rule") + } + if got != want { + t.Errorf("describe rule M.R = %q, want %q", got, want) + } + if _, errs := visitor.Build(want); len(errs) > 0 { + t.Fatalf("%q does not parse: %v", want, errs) + } + if got := objectTypeToDescribe["RULE"]; got != "rule" { + t.Errorf("catalog ObjectType RULE auto-detects as %q, want rule", got) + } + if got := unitTypeToDescribe["Microflows$Rule"]; got != "rule" { + t.Errorf("unit type Microflows$Rule auto-detects as %q, want rule", got) + } + if !strings.Contains(describeCmd.Long, "\n rule ") { + t.Error("describe --help lists no rule type line") + } +} From e9024d91aef17b3f5c7b0b332934bf4e273248aa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:30:25 +0000 Subject: [PATCH 04/21] fix: install names `marketplace update` for a module already present (mendixlabs/mxcli#1442) Reported: `marketplace install` on a module already in the project says updates are "NOT done automatically" and hands the user off to Studio Pro, in a container that has none; `mxcli marketplace update` exists. Install still refuses to update implicitly, but now prints the concrete `mxcli marketplace diff -p --to ` and `mxcli marketplace update -p --to ` commands (placeholders for a --file install, which has no content id). Long help and the marketplace guide / capabilities table say the same. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- cmd/mxcli/cmd_marketplace_install.go | 50 ++++++++++++++----- ...md_marketplace_install_update_hint_test.go | 40 +++++++++++++++ docs-site/src/guides/marketplace.md | 14 +++--- docs-site/src/reference/capabilities.md | 2 +- 4 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 cmd/mxcli/cmd_marketplace_install_update_hint_test.go diff --git a/cmd/mxcli/cmd_marketplace_install.go b/cmd/mxcli/cmd_marketplace_install.go index c675e14fb..f7bd25257 100644 --- a/cmd/mxcli/cmd_marketplace_install.go +++ b/cmd/mxcli/cmd_marketplace_install.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" modelsdk "github.com/mendixlabs/mxcli" @@ -32,9 +33,12 @@ Install is type-aware: - Module copied in with mxcli's own writer (new modules only) - other types downloaded to disk with import instructions -Updating a module that is already present is NOT done automatically: it could +Updating a module that is already present is NOT done by install: it could discard local edits and, for modules with persistent entities, change entity -IDs (which loses data). Such updates are reported and left to Studio Pro. +IDs (which loses data). Install reports the module and names the command that +updates it — 'marketplace diff' to see what an update would touch, then +'marketplace update -p --to ', which carries +the entity identity and role grants across. A module is installed by copying its units with mxcli's own writer rather than by running 'mx module-import'. That preserves the project's storage format — @@ -142,7 +146,7 @@ func runMarketplaceInstall(cmd *cobra.Command, args []string) error { return err } } - return installModule(cmd.Context(), client, version, mprPath, allowFormatChange, out) + return installModule(cmd.Context(), client, contentID, version, mprPath, allowFormatChange, out) default: // Theme / Starter App / Sample / unknown: download + instruct rather // than guess a placement we can't verify. @@ -170,7 +174,7 @@ func installFromFile(ctx context.Context, mpkPath, mprPath string, allowFormatCh } switch { case pkg.ModelerProject.Module.Name != "": - return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, "", "", + return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, 0, "", "", "from "+filepath.Base(mpkPath), out) case pkg.ClientModule.Name != "": return placeWidgetFile(mpkPath, filepath.Dir(mprPath), out) @@ -221,7 +225,7 @@ func printWidgetNext(out io.Writer) { // installModule imports a module .mpk into the project, but only when the module // is not already present. An existing module is reported, not modified. -func installModule(ctx context.Context, client *marketplace.Client, v *marketplace.Version, mprPath string, allowFormatChange bool, out io.Writer) error { +func installModule(ctx context.Context, client *marketplace.Client, contentID int, v *marketplace.Version, mprPath string, allowFormatChange bool, out io.Writer) error { // Download to a temp .mpk so we can inspect it and hand it to mx. tmpDir, err := os.MkdirTemp("", "mxcli-install-") if err != nil { @@ -232,7 +236,7 @@ func installModule(ctx context.Context, client *marketplace.Client, v *marketpla if err != nil { return err } - return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, v.VersionNumber, v.VersionID, + return installModuleFromFile(ctx, mpkPath, mprPath, allowFormatChange, contentID, v.VersionNumber, v.VersionID, "version "+v.VersionNumber, out) } @@ -240,9 +244,10 @@ func installModule(ctx context.Context, client *marketplace.Client, v *marketpla // package is on disk — shared by the online path (after its download) and by // --file (which has no download). versionNumber and versionID are the // marketplace identity to record on the module; both are empty for a package -// from disk, which has none. label is how the package is named in output. +// from disk, which has none, and so is contentID (0). label is how the package +// is named in output. func installModuleFromFile(ctx context.Context, mpkPath, mprPath string, allowFormatChange bool, - versionNumber, versionID, label string, out io.Writer) error { + contentID int, versionNumber, versionID, label string, out io.Writer) error { moduleName, err := moduleNameFromMpk(mpkPath) if err != nil { @@ -259,11 +264,8 @@ func installModuleFromFile(ctx context.Context, mpkPath, mprPath string, allowFo _ = reader.Disconnect() if existing { - // Postponed: do NOT auto-update modules — see the module-update memory. - fmt.Fprintf(out, "Module %q is already installed (version %s).\n", moduleName, displayVer(installedVer)) - fmt.Fprintf(out, "Target: %s.\n", label) - fmt.Fprintln(out, "In-place module updates are not applied automatically (they can discard local") - fmt.Fprintln(out, "edits and change persistent-entity IDs, which loses data). Update via Studio Pro.") + // Install never updates implicitly: an update is its own command. + reportModuleAlreadyInstalled(out, moduleName, installedVer, label, contentID, versionNumber, mprPath) return nil } @@ -492,3 +494,25 @@ func moduleNameFromMpk(mpkPath string) (string, error) { } return "", fmt.Errorf("package.xml has no module name") } + +// reportModuleAlreadyInstalled says why install left a present module alone and +// names the command that updates it (mendixlabs/mxcli#1442). It used to send +// the user to Studio Pro, although 'marketplace update' does the update +// headlessly and carries the GUIDs and role grants a plain replace would lose. +// contentID and versionNumber are 0/"" for a --file install, which has neither; +// the command is then named with placeholders. +func reportModuleAlreadyInstalled(out io.Writer, moduleName, installedVer, label string, contentID int, versionNumber, mprPath string) { + id, to := "", "" + if contentID > 0 { + id = strconv.Itoa(contentID) + } + if versionNumber != "" { + to = versionNumber + } + fmt.Fprintf(out, "Module %q is already installed (version %s).\n", moduleName, displayVer(installedVer)) + fmt.Fprintf(out, "Target: %s.\n", label) + fmt.Fprintln(out, "It is not updated by install (an update can discard local edits). To update it,") + fmt.Fprintln(out, "review what would change, then apply it with entity identity and role grants preserved:") + fmt.Fprintf(out, " mxcli marketplace diff %s -p %s --to %s\n", id, mprPath, to) + fmt.Fprintf(out, " mxcli marketplace update %s -p %s --to %s\n", id, mprPath, to) +} diff --git a/cmd/mxcli/cmd_marketplace_install_update_hint_test.go b/cmd/mxcli/cmd_marketplace_install_update_hint_test.go new file mode 100644 index 000000000..a26f9c1d6 --- /dev/null +++ b/cmd/mxcli/cmd_marketplace_install_update_hint_test.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "strings" + "testing" +) + +// An install of a module that is already present still refuses to update it, +// but names the command that does (mendixlabs/mxcli#1442) rather than handing +// the user off to Studio Pro, which a headless environment does not have. +func TestReportModuleAlreadyInstalled_PointsToMarketplaceUpdate(t *testing.T) { + var out bytes.Buffer + reportModuleAlreadyInstalled(&out, "Administration", "4.3.2", "version 4.5.0", 23513, "4.5.0", "X/X.mpr") + got := out.String() + for _, want := range []string{ + `Module "Administration" is already installed (version 4.3.2).`, + "not updated by install", + "mxcli marketplace diff 23513 -p X/X.mpr --to 4.5.0", + "mxcli marketplace update 23513 -p X/X.mpr --to 4.5.0", + } { + if !strings.Contains(got, want) { + t.Errorf("missing %q in:\n%s", want, got) + } + } + if strings.Contains(got, "Update via Studio Pro") { + t.Errorf("still hands off to Studio Pro:\n%s", got) + } + + // --file has no content id or marketplace version: the command is named + // with placeholders instead of invented values. + out.Reset() + reportModuleAlreadyInstalled(&out, "CompanyTheme", "", "from CompanyTheme.mpk", 0, "", "app.mpr") + got = out.String() + if !strings.Contains(got, "mxcli marketplace update -p app.mpr --to ") { + t.Errorf("--file install: no update command named:\n%s", got) + } +} diff --git a/docs-site/src/guides/marketplace.md b/docs-site/src/guides/marketplace.md index 97242be17..db36673d8 100644 --- a/docs-site/src/guides/marketplace.md +++ b/docs-site/src/guides/marketplace.md @@ -131,23 +131,25 @@ If you take that route the command states plainly that the project is now v1. ## Updating an existing module -Updating a module that is **already in the project is not done automatically**. `install` detects it, reports the installed and target versions, and stops: +Updating a module that is **already in the project is not done by `install`**. `install` detects it, reports the installed and target versions, names the command that does the update, and stops: ```text Module "DatabaseConnector" is already installed (version 7.0.1). -Target version: 7.0.3. -In-place module updates are not applied automatically (they can discard local -edits and change persistent-entity IDs, which loses data). Update via Studio Pro. +Target: version 7.0.3. +It is not updated by install (an update can discard local edits). To update it, +review what would change, then apply it with entity identity and role grants preserved: + mxcli marketplace diff 2888 -p app.mpr --to 7.0.3 + mxcli marketplace update 2888 -p app.mpr --to 7.0.3 ``` -Before you update in Studio Pro, the question worth answering is **whether anyone has edited the module since it was installed** — because the update will not ask. That is what `marketplace diff` is for; see below. +Before you update, the question worth answering is **whether anyone has edited the module since it was installed** — because the update will not ask. That is what `marketplace diff` is for; see below. Two reasons make automatic in-place module updates unsafe: 1. **Local edits.** Teams sometimes modify a marketplace module after importing it; a blind re-import would discard those changes. 2. **Persistent-entity IDs.** A fresh import assigns new entity `$ID`s. The runtime database keys data by entity ID, so re-importing a module with persistent entities would make the runtime treat them as *different* entities — **losing data**. -Studio Pro's Marketplace **Update** performs an ID-preserving merge that the `mx` CLI does not expose, so module updates are left to Studio Pro for now. +Studio Pro's Marketplace **Update** performs an ID-preserving merge that the `mx` CLI does not expose; `marketplace update` (below) is mxcli's equivalent, as a separate command so that `install` never updates implicitly. ## Updating a module (`marketplace update`) diff --git a/docs-site/src/reference/capabilities.md b/docs-site/src/reference/capabilities.md index 790bf5f53..4f7ec1cb0 100644 --- a/docs-site/src/reference/capabilities.md +++ b/docs-site/src/reference/capabilities.md @@ -144,7 +144,7 @@ Everything mxcli can do, organized by use case. | REST query parameters | Requires Mendix 11.0+ | Build query string manually on 10.x | | Concurrent editing | Not supported — a file-based write is refused while Studio Pro has the project open (`.mpr.lock` present) | Close Studio Pro before mxcli writes, or write through it with `--mcp`; `exec --force` overrides | | Widget template drift | CE0463 on version mismatch | MPK augmentation handles most cases | -| Marketplace module update | Existing modules are reported, not updated in place | Update via Studio Pro (preserves local edits and entity IDs) | +| Marketplace module update | `install` reports an existing module and does not update it | `mxcli marketplace diff` / `update -p app.mpr --to ` (preserves entity identity and role grants; local edits are refused or saved with `--save-edits`) | | 47 of 52 metamodel domains | Not yet implemented | REST, OData write, etc. pending | ## Version Compatibility From 7de5fbb21250b7774be4519775f66e7eba17d069 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:38:12 +0000 Subject: [PATCH 05/21] fix: fail a statement whose output hit the line limit (mendixlabs/mxcli#1425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: "`describe module ... with all` truncates at 10 000 lines without a warning" — "output stops in the middle of a statement", exit 0. The per-statement output guard returns an error from Write once it passes maxOutputLines and discards everything after, but describers ignore Fprintf's error, so the handler returned nil and Execute never looked at the guard. Execute now returns the limit error when the statement otherwise succeeded but the guard was exceeded; the message says the output was truncated and suggests describing per document. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...2-output-line-limit-silent-truncation.json | 1 + mdl/executor/executor.go | 7 +++ mdl/executor/output_guard.go | 21 +++++++- mdl/executor/output_guard_test.go | 54 +++++++++++++++++++ 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json create mode 100644 mdl/executor/output_guard_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json new file mode 100644 index 000000000..c0f8d817f --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-output-line-limit-silent-truncation.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1425: `describe module ... with all` truncates at 10 000 lines without a warning — output stops in the middle of a statement, exit 0.", "cause": "outputGuard.Write returns an error once the per-statement limit (maxOutputLines, 10_000) is passed and silently discards everything after, but every describer ignores Fprintf's error, so the handler returned nil and Executor.Execute never looked at guard.exceeded.", "file": "mdl/executor/output_guard.go; mdl/executor/executor.go", "fix": "Execute: if the statement otherwise succeeded but the guard was exceeded, return the limit error (now saying the output was truncated, with a hint to describe per document). No knob raises the limit; none was added.", "test": "TestExecuteReportsOutputLineLimitExceeded (handler writing maxOutputLines lines is the control)", "insight": "**An error returned from io.Writer.Write is not a reporting channel** — nearly all output code ignores it. A guard that signals through Write must also be checked by whoever owns the statement's result. The tell for this class is an output length that is exactly a round constant: grep the number before suspecting the describer."} diff --git a/mdl/executor/executor.go b/mdl/executor/executor.go index 7b71fee34..f7c389d47 100644 --- a/mdl/executor/executor.go +++ b/mdl/executor/executor.go @@ -417,6 +417,13 @@ func (e *Executor) Execute(stmt ast.Statement) error { } } + // A handler that ignored the guard's Write error still "succeeded" with its + // output cut off at the limit; that must fail the statement, not exit 0 + // (mendixlabs/mxcli#1425). + if err == nil && e.guard != nil { + err = e.guard.exceededError() + } + if e.logger != nil { e.logger.Command(stmtTypeName(stmt), stmtSummary(stmt), time.Since(start), err) } diff --git a/mdl/executor/output_guard.go b/mdl/executor/output_guard.go index f0ae98989..f9dc5bd17 100644 --- a/mdl/executor/output_guard.go +++ b/mdl/executor/output_guard.go @@ -48,8 +48,27 @@ func (g *outputGuard) Write(p []byte) (int, error) { g.exceeded = true // Write the current chunk first so output isn't abruptly cut mid-line. _, _ = g.w.Write(p) - return len(p), mdlerrors.NewValidationf("output line limit exceeded (%d lines); statement aborted", g.maxLines) + return len(p), g.limitError() } return g.w.Write(p) } + +// exceededError returns the limit error if the current statement went past the +// line limit, or nil. Execute checks it after the statement returns: most +// handlers (every describer) ignore Fprintf's error, so the one Write returned +// never reaches the caller, and without this check the statement succeeded +// with its output silently cut off (mendixlabs/mxcli#1425). +func (g *outputGuard) exceededError() error { + g.mu.Lock() + defer g.mu.Unlock() + if !g.exceeded { + return nil + } + return g.limitError() +} + +func (g *outputGuard) limitError() error { + return mdlerrors.NewValidationf("output line limit exceeded (%d lines); output truncated and statement aborted. "+ + "Describe a large module per document (e.g. describe entity / microflow / page ...) instead of all at once", g.maxLines) +} diff --git a/mdl/executor/output_guard_test.go b/mdl/executor/output_guard_test.go new file mode 100644 index 000000000..ec040e921 --- /dev/null +++ b/mdl/executor/output_guard_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// A statement whose output passes the per-statement line limit must fail, not +// exit 0 with the text cut off mid-statement. Describers ignore Fprintf's +// error, so the guard's own error never reached the caller and +// `describe module … with all` stopped at exactly 10,000 lines with no +// warning (mendixlabs/mxcli#1425). The statement under the limit is the +// control. +func TestExecuteReportsOutputLineLimitExceeded(t *testing.T) { + for _, tc := range []struct { + name string + lines int + wantErr bool + }{ + {"under the limit", maxOutputLines, false}, + {"over the limit", maxOutputLines + 500, true}, + } { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + e := New(&out) + e.registry.handlers[reflect.TypeOf(&ast.DescribeStmt{})] = func(ctx *ExecContext, _ ast.Statement) error { + for i := 0; i < tc.lines; i++ { + fmt.Fprintf(ctx.Output, "line %d\n", i) // error ignored, as describers do + } + return nil + } + err := e.Execute(&ast.DescribeStmt{}) + if !tc.wantErr { + if err != nil { + t.Fatalf("Execute: %v", err) + } + return + } + if err == nil { + t.Fatalf("Execute returned nil after %d lines; output silently truncated at %d", tc.lines, strings.Count(out.String(), "\n")) + } + if !strings.Contains(err.Error(), "truncated") { + t.Fatalf("error does not say the output was truncated: %v", err) + } + }) + } +} From a9cec305666cf9602233cdac3dfd4bb7717aca20 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:41:12 +0000 Subject: [PATCH 06/21] fix: describe code action writes param separator before its doc comment (mendixlabs/mxcli#1412) Reported: describe java action prints `Name: String not null -- This sets the Name of the tool...,` "the comma lands inside the comment, so the next parameter is a syntax error" -- mxcli check of the output fails with `line 255:4 mismatched input 'Name' expecting ')'`. The parameter loop wrote ", " at the start of the next iteration, i.e. after the previous parameter's trailing `-- doc` comment. The JavaScript action describer carries the same loop and the same bug. Both now write the separator at the end of each non-final parameter, before the comment. Test: describe -> visitor.Build round trip for a Java and a JavaScript action with documented parameters; both fail with the fix reverted (`mismatched input 'Amount' expecting ')'`). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...ion-param-separator-after-doc-comment.json | 1 + mdl/executor/cmd_javaactions.go | 11 ++- mdl/executor/cmd_javascript_actions.go | 11 ++- ...scribe_code_action_param_separator_test.go | 71 +++++++++++++++++++ 4 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json create mode 100644 mdl/executor/describe_code_action_param_separator_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json new file mode 100644 index 000000000..fdf578274 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-java-action-param-separator-after-doc-comment.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "`describe java action` on an action with a documented parameter printed `Name: String not null -- This sets the Name of the tool...,` so the comma sat inside the comment and `mxcli check` of the output failed `line 255:4 mismatched input 'Name' expecting ')'`.", "cause": "The parameter loop wrote the `, ` separator at the start of the next iteration, i.e. after the previous parameter's trailing `-- doc` comment. The JavaScript action describer is a copy of the same loop and had the same bug.", "fix": "Write the separator at the end of each non-final parameter, before its `--` comment (cmd_javaactions.go and cmd_javascript_actions.go).", "insight": "Any describer that emits a trailing `--` comment must emit list separators before it; a separator written 'before the next item' is only safe when nothing line-terminating can come between. grep `\" -- \"` in mdl/executor for other list emitters. A describe->visitor.Build round trip catches it; a substring assertion on the describe text does not. Avoid naming a test parameter `Count`: it is an MDL keyword and the parser's keyword hint then masks the real error.", "issue": "mendixlabs/mxcli#1412", "file": "mdl/executor/cmd_javaactions.go, mdl/executor/cmd_javascript_actions.go (parameter loop)", "test": "mdl/executor/describe_code_action_param_separator_test.go TestDescribeJavaAction_ParamDocCommentKeepsSeparator, TestDescribeJavaScriptAction_ParamDocCommentKeepsSeparator; control: both fail `mismatched input 'Amount' expecting ')'` with the fix reverted"} diff --git a/mdl/executor/cmd_javaactions.go b/mdl/executor/cmd_javaactions.go index 0aa66181f..ceba36366 100644 --- a/mdl/executor/cmd_javaactions.go +++ b/mdl/executor/cmd_javaactions.go @@ -105,9 +105,6 @@ func describeJavaAction(ctx *ExecContext, name ast.QualifiedName) error { } for i, param := range ja.Parameters { - if i > 0 { - sb.WriteString(", ") - } if hasParamDescriptions { sb.WriteString("\n ") } @@ -121,6 +118,14 @@ func describeJavaAction(ctx *ExecContext, name ast.QualifiedName) error { if param.IsRequired { sb.WriteString(" not null") } + // The separator precedes any `--` doc comment: written after it, the + // comment swallows the comma and the next parameter fails to parse (#1412). + if i < len(ja.Parameters)-1 { + sb.WriteString(",") + if !hasParamDescriptions { + sb.WriteString(" ") + } + } if param.Description != "" { paramDoc := strings.ReplaceAll(param.Description, "\r\n", "\n") paramDoc = strings.ReplaceAll(paramDoc, "\r", "\n") diff --git a/mdl/executor/cmd_javascript_actions.go b/mdl/executor/cmd_javascript_actions.go index c771659cf..db1f07d9c 100644 --- a/mdl/executor/cmd_javascript_actions.go +++ b/mdl/executor/cmd_javascript_actions.go @@ -118,9 +118,6 @@ func describeJavaScriptAction(ctx *ExecContext, name ast.QualifiedName) error { } for i, param := range jsa.Parameters { - if i > 0 { - sb.WriteString(", ") - } if hasParamDescriptions { sb.WriteString("\n ") } @@ -134,6 +131,14 @@ func describeJavaScriptAction(ctx *ExecContext, name ast.QualifiedName) error { if param.IsRequired { sb.WriteString(" not null") } + // The separator precedes any `--` doc comment: written after it, the + // comment swallows the comma and the next parameter fails to parse (#1412). + if i < len(jsa.Parameters)-1 { + sb.WriteString(",") + if !hasParamDescriptions { + sb.WriteString(" ") + } + } if param.Description != "" { paramDoc := strings.ReplaceAll(param.Description, "\r\n", "\n") paramDoc = strings.ReplaceAll(paramDoc, "\r", "\n") diff --git a/mdl/executor/describe_code_action_param_separator_test.go b/mdl/executor/describe_code_action_param_separator_test.go new file mode 100644 index 000000000..452d68aa9 --- /dev/null +++ b/mdl/executor/describe_code_action_param_separator_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/javaactions" +) + +// documentedParams has a description on non-final parameters: describe +// prints each as a trailing `--` comment, so the separator must come before it. +func documentedParams() []*types.JavaActionParameter { + return []*types.JavaActionParameter{ + {Name: "Name", Description: "This sets the Name of the tool.", IsRequired: true, ParameterType: &types.StringType{}}, + {Name: "Amount", Description: "How many.", ParameterType: &types.IntegerType{}}, + {Name: "Extra", ParameterType: &types.StringType{}}, + } +} + +// mendixlabs/mxcli#1412: the separator was written at the start of the next +// parameter, i.e. after the previous one's `-- doc` comment, so the comment +// swallowed the comma and the next parameter failed to parse. +func TestDescribeJavaAction_ParamDocCommentKeepsSeparator(t *testing.T) { + mod := mkModule("MyModule") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ReadJavaActionByNameFunc: func(string) (*javaactions.JavaAction, error) { + return &javaactions.JavaAction{ + BaseElement: model.BaseElement{ID: nextID("ja")}, + ContainerID: mod.ID, + Name: "CreateTool", + Parameters: documentedParams(), + }, nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + assertNoError(t, describeJavaAction(ctx, ast.QualifiedName{Module: "MyModule", Name: "CreateTool"})) + out := buf.String() + stmt := findStmt[*ast.CreateJavaActionStmt](t, reparse(t, out), out) + if len(stmt.Parameters) != 3 { + t.Fatalf("re-parsed %d parameters, want 3\n%s", len(stmt.Parameters), out) + } +} + +func TestDescribeJavaScriptAction_ParamDocCommentKeepsSeparator(t *testing.T) { + mod := mkModule("WebMod") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ReadJavaScriptActionByNameFunc: func(string) (*types.JavaScriptAction, error) { + return &types.JavaScriptAction{ + BaseElement: model.BaseElement{ID: nextID("jsa")}, + ContainerID: mod.ID, + Name: "CreateTool", + Platform: "Web", + Parameters: documentedParams(), + }, nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + assertNoError(t, describeJavaScriptAction(ctx, ast.QualifiedName{Module: "WebMod", Name: "CreateTool"})) + out := buf.String() + stmt := findStmt[*ast.CreateJavaScriptActionStmt](t, reparse(t, out), out) + if len(stmt.Parameters) != 3 { + t.Fatalf("re-parsed %d parameters, want 3\n%s", len(stmt.Parameters), out) + } +} From 7077c8509e26758011d5e891ccd79d933c3b83ed Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:42:30 +0000 Subject: [PATCH 07/21] fix: refuse alter settings JavaVersion on a model that stores none (mendixlabs/mxcli#1438) Reported: `alter settings runtime ( JavaVersion: '17' );` on a Mendix 9.24 model printed "Unchanged model settings" -- no hint that the property does not exist in that version. refuseIfNotStored exempted JavaVersion unconditionally because it is stored under either JavaVersion or JavaMajorVersion. A 9.x document carries neither, and settingsoverlay.SetJavaVersion writes nothing when JavaVersionKey(raw) is empty, so the statement was a silent no-op. Exempt only when one of the keys is present; otherwise refuse, saying the setting does not exist on this Mendix version and the JDK is chosen by the platform. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...alter-settings-javaversion-noop-on-9x.json | 1 + mdl/executor/cmd_settings.go | 18 ++++++-- mdl/executor/cmd_settings_model_test.go | 43 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json new file mode 100644 index 000000000..1a745e655 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-alter-settings-javaversion-noop-on-9x.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-12", "symptom": "`alter settings runtime ( JavaVersion: '17' )` on a Mendix 9.24 model printed \"Unchanged model settings\" — a silent no-op instead of an error that the property does not exist on that version", "cause": "`refuseIfNotStored` exempted JavaVersion unconditionally because it is stored under one of two keys (JavaVersion / JavaMajorVersion), but a 9.x document carries neither, and `settingsoverlay.SetJavaVersion` writes nothing when `JavaVersionKey(raw)==\"\"`", "file": "`mdl/executor/cmd_settings.go` (`refuseIfNotStored`)", "insight": "An exemption from a presence gate written for 'stored under an alias' must still check that some alias is present: the alias resolver returning \"\" is the not-stored case. DESCRIBE already gated on `JavaVersionKey != \"\"`; the ALTER path had not been made symmetric. Revert proof: MockBackend with a 9.24-shaped part (no Java key) returns nil from alterSettings", "refs": ["mendixlabs/mxcli#1438"]} diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index 8f530c071..73db42a39 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -359,12 +359,24 @@ func storedModelSettings(ps *model.ProjectSettings) map[string]any { // property the type may not define), so without this check the statement would // report success and change nothing — the silent no-op shape of #805. // -// JavaVersion is exempt: it is stored under either JavaVersion or -// JavaMajorVersion, and settingsoverlay.JavaVersionKey resolves which. +// JavaVersion is stored under either JavaVersion or JavaMajorVersion, and +// settingsoverlay.JavaVersionKey resolves which. A document carrying neither +// (Mendix 9.x) has no Java version setting at all, and the overlay would write +// nothing — mendixlabs/mxcli#1438 reported "Unchanged model settings" there. func refuseIfNotStored(raw map[string]any, key string) error { - if raw == nil || key == "JavaVersion" { + if raw == nil { return nil } + if key == "JavaVersion" { + if settingsoverlay.JavaVersionKey(raw) != "" { + return nil + } + return mdlerrors.NewUnsupported( + "this project does not store the model setting JavaVersion\n" + + " The JavaVersion setting does not exist on this project's Mendix version " + + "(Mendix 9.x has none); the JDK is chosen by the platform.\n" + + " hint: upgrade the project to choose a Java version in the model") + } if settingsoverlay.Has(raw, key) { return nil } diff --git a/mdl/executor/cmd_settings_model_test.go b/mdl/executor/cmd_settings_model_test.go index 1ffa36dca..eba447f8d 100644 --- a/mdl/executor/cmd_settings_model_test.go +++ b/mdl/executor/cmd_settings_model_test.go @@ -276,6 +276,49 @@ func TestAlterSettingsModel_RefusesPropertyThisVersionDoesNotStore(t *testing.T) } } +// TestAlterSettingsModel_JavaVersionRefusedWhenNotStored is mendixlabs/mxcli#1438: +// a Mendix 9.x document stores neither JavaVersion nor JavaMajorVersion, so the +// overlay has no key to write and `alter settings runtime (JavaVersion: '17')` +// reported "Unchanged model settings". It must refuse instead. +func TestAlterSettingsModel_JavaVersionRefusedWhenNotStored(t *testing.T) { + var written *model.ProjectSettings + // A Mendix 9.24-shaped document: no Java version key at all. + ctx, _ := newMockCtx(t, withBackend(modelSettingsBackend(&written, "HashAlgorithm", "BcryptCost"))) + + err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"JavaVersion": "17"}, + }) + if err == nil { + t.Fatal("expected a refusal: this project stores no JavaVersion setting, got nil (silent no-op)") + } + if !strings.Contains(err.Error(), "JavaVersion") { + t.Errorf("error should name the property, got: %v", err) + } + if written != nil { + t.Error("a refused statement must not write") + } +} + +// The JavaVersion exemption still applies when either storage key is present. +func TestAlterSettingsModel_JavaVersionAllowedUnderEitherKey(t *testing.T) { + for _, key := range []string{"JavaVersion", "JavaMajorVersion"} { + t.Run(key, func(t *testing.T) { + var written *model.ProjectSettings + ctx, _ := newMockCtx(t, withBackend(modelSettingsBackend(&written, key))) + if err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "model", + Properties: map[string]any{"JavaVersion": "21"}, + }); err != nil { + t.Fatalf("alterSettings: %v", err) + } + if written == nil { + t.Fatal("no settings written") + } + }) + } +} + // The gate must not block a property the document does carry. func TestAlterSettingsModel_AllowsStoredProperty(t *testing.T) { var written *model.ProjectSettings From 43dccb292c74550db929f7e47f46d84843bf0e93 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:51:02 +0000 Subject: [PATCH 08/21] fix: declare an aggregate's output variable in the build pass (mendixlabs/mxcli#1413) Reported: `check` ends with `Check passed!`, but `exec` refuses $Sum = sum $List by Amount; set $Sum = $Sum + 1; with "variable 'Sum' is not declared" (same for `$N = count $List`). The validate pass declared an AggregateListStmt's output; addAggregateListAction, which exec runs, never registered it, so the SET's declared-variable check failed only at exec. It now declares the result: count Integer, average Decimal, all/any Boolean, reduce its declared return type, and sum/min/max Unknown (their type follows the attribute, which is not resolved here). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...12-set-on-aggregate-output-undeclared.json | 1 + .../1413-set-on-aggregate-output.mdl | 31 ++++++++++ .../cmd_microflows_builder_actions.go | 20 +++++++ ...roflows_builder_aggregate_declared_test.go | 57 +++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json create mode 100644 mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl create mode 100644 mdl/executor/cmd_microflows_builder_aggregate_declared_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json new file mode 100644 index 000000000..4d0b347bb --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-set-on-aggregate-output-undeclared.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1413: `$Sum = sum $List by Amount; set $Sum = $Sum + 1;` (also count) passed `check -p` but `exec` refused the microflow with \"variable 'Sum' is not declared\".", "cause": "Two passes keep their own declared-variable bookkeeping. validateStatements declared an AggregateListStmt's output as Unknown; addAggregateListAction (the build pass exec runs) never put it in declaredVars or varTypes, so addChangeVariableAction's isVariableDeclared check failed only at exec.", "file": "mdl/executor/cmd_microflows_builder_actions.go", "fix": "addAggregateListAction declares the output: count Integer, average Decimal, all/any Boolean, reduce its declared return type, sum/min/max Unknown (attribute-typed, not resolved here).", "test": "TestAggregateOutputIsDeclaredForSet; mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl", "insight": "**check passing while exec refuses is a validate-vs-build bookkeeping split**: diff the statement's case in cmd_microflows_builder_validate.go against its add*Action builder — whichever registers declaredVars/varTypes where the other does not is the bug. A builder unit test driving buildFlowGraph with the producing statement followed by a SET reproduces it with no project, and the control (fix reverted) fails with the exact reported message. Worth checking the same shape in other output-producing builders: addListOperationAction registers only list/object outputs in varTypes, so a Boolean CONTAINS/EQUALS output is likewise undeclared in the build pass."} diff --git a/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl b/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl new file mode 100644 index 000000000..409511c3f --- /dev/null +++ b/mdl-examples/bug-tests/1413-set-on-aggregate-output.mdl @@ -0,0 +1,31 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1413: SET on a variable an aggregate produced +-- ============================================================================ +-- +-- `check -p` passed, `exec` refused the microflow with +-- +-- variable 'Sum' is not declared +-- +-- The validate pass declared an aggregate's output variable; the build pass +-- (addAggregateListAction) never did, so the SET that followed it failed only +-- at exec time. Both microflows below must exec cleanly. +-- ============================================================================ + +create module Bug1413; + +create persistent entity Bug1413.EntA ( Amount: Integer ); + +create microflow Bug1413.MF_Sum ($List: list of Bug1413.EntA) returns Integer +begin + $Sum = sum $List by Amount; + set $Sum = $Sum + 1; + return $Sum; +end; + +create microflow Bug1413.MF_Count ($List: list of Bug1413.EntA) returns Integer +begin + $N = count $List; + set $N = $N + 1; + return $N; +end; diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 172e6d0e5..87c541301 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -2012,6 +2012,26 @@ func (fb *flowBuilder) addAggregateListAction(s *ast.AggregateListStmt) model.ID } } + // Declare the result so a later SET on it is accepted, as the validate path + // already does (mendixlabs/mxcli#1413). SUM/MIN/MAX follow the attribute's + // type, which is not resolved here, so they stay Unknown. + if fb.declaredVars != nil && s.OutputVariable != "" { + resultType := "Unknown" + switch s.Operation { + case ast.AggregateCount: + resultType = "Integer" + case ast.AggregateAverage: + resultType = "Decimal" + case ast.AggregateAll, ast.AggregateAny: + resultType = "Boolean" + case ast.AggregateReduce: + if s.ReturnType != nil { + resultType = s.ReturnType.Kind.String() + } + } + fb.declaredVars[s.OutputVariable] = resultType + } + activity := µflows.ActionActivity{ BaseActivity: microflows.BaseActivity{ BaseMicroflowObject: microflows.BaseMicroflowObject{ diff --git a/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go b/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go new file mode 100644 index 000000000..9c7aa0dca --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_aggregate_declared_test.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1413: `$Sum = sum $List by Amount; set $Sum = $Sum + 1;` +// passed `check -p` but `exec` refused it with "variable 'Sum' is not +// declared" — the build path never registered an aggregate's output variable, +// while the validate path did. +func TestAggregateOutputIsDeclaredForSet(t *testing.T) { + cases := []struct { + name string + stmt *ast.AggregateListStmt + want string // declaredVars type, "" = only require it be declared + }{ + {"sum", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateSum, InputVariable: "List", Attribute: "Amount"}, ""}, + {"count", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateCount, InputVariable: "List"}, "Integer"}, + {"average", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateAverage, InputVariable: "List", Attribute: "Amount"}, "Decimal"}, + {"maximum", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateMaximum, InputVariable: "List", Attribute: "Amount"}, ""}, + {"any", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateAny, InputVariable: "List", + IsExpression: true, Expression: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}}, "Boolean"}, + {"reduce", &ast.AggregateListStmt{OutputVariable: "Sum", Operation: ast.AggregateReduce, InputVariable: "List", + IsExpression: true, Expression: &ast.VariableExpr{Name: "currentResult"}, + InitialValue: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: int64(0)}, + ReturnType: &ast.DataType{Kind: ast.TypeInteger}}, "Integer"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + body := []ast.MicroflowStatement{ + tc.stmt, + &ast.MfSetStmt{Target: "Sum", Value: &ast.BinaryExpr{ + Left: &ast.VariableExpr{Name: "Sum"}, + Operator: "+", + Right: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: int64(1)}, + }}, + } + fb := &flowBuilder{ + posX: 200, posY: 200, baseY: 200, spacing: HorizontalSpacing, + varTypes: map[string]string{"List": "List of ModA.EntA"}, + declaredVars: map[string]string{}, + } + fb.buildFlowGraph(body, nil) + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("build: %s", strings.Join(errs, "; ")) + } + if tc.want != "" && fb.declaredVars["Sum"] != tc.want { + t.Errorf("declared type = %q, want %q", fb.declaredVars["Sum"], tc.want) + } + }) + } +} From 6ed5f12060bbdda9c6af4719bcfdbc77f93f9991 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:55:41 +0000 Subject: [PATCH 09/21] fix: unquote quoted names in OData expose clauses (mendixlabs/mxcli#1423) Reported: `expose (Name as 'Name' (KEY), "Description")` passes check and exec, then mx check fails with [CE1613] "The selected attribute 'ModA.EntA."Description"' no longer exists." and describe prints `"Description" as '"Description"'`. parseExposeMembers and the publish-microflow expose loop read IdentifierOrKeyword().GetText(), which keeps a QUOTED_IDENTIFIER's quotes. Use identifierOrKeywordText, as every other name slot in the file does. After the fix the repro describes as `Description as 'Description'`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...10-12-odata-expose-quoted-member-name.json | 1 + .../1423-odata-expose-quoted-member-name.mdl | 29 ++++++++++++++++ mdl/visitor/visitor_odata.go | 4 +-- .../visitor_odata_quoted_member_test.go | 33 +++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json create mode 100644 mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl create mode 100644 mdl/visitor/visitor_odata_quoted_member_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json new file mode 100644 index 000000000..8f1466240 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-12-odata-expose-quoted-member-name.json @@ -0,0 +1 @@ +{"area":"mdl/visitor","date":"2026-10-12","symptom":"published OData `expose (Name as 'Name' (KEY), \"Description\")` stored the member name with its double quotes; check and exec were clean, describe printed `\"Description\" as '\"Description\"'`, mx check reported CE1613 \"The selected attribute 'ModA.EntA.\"Description\"' no longer exists\" (mendixlabs/mxcli#1423). Published microflow parameters had the same defect","cause":"parseExposeMembers and the publish-microflow expose loop took `member.IdentifierOrKeyword().GetText()`, which keeps a QUOTED_IDENTIFIER's quotes; every other identifierOrKeyword slot in the file already used identifierOrKeywordText","file":"`mdl/visitor/visitor_odata.go` (parseExposeMembers, publish microflow expose)","insight":"**`identifierOrKeyword().GetText()` is the raw token — quotes included.** Any name slot reading it bypasses the quoting the skills recommend, and nothing downstream notices because the quoted string is still a valid-looking name until mx resolves it. `grep -n 'IdentifierOrKeyword().GetText()' mdl/visitor` is the audit; the remaining hits are enum/keyword values or already wrapped in unquoteIdentifier. Describe round-trip is the cheapest repro: the stored quotes reappear inside the `as '...'` exposed name","refs":["mendixlabs/mxcli#1423"],"ce":["CE1613"]} diff --git a/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl new file mode 100644 index 000000000..219963087 --- /dev/null +++ b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl @@ -0,0 +1,29 @@ +mdl 1; + +/** + * mendixlabs/mxcli#1423: published OData `expose ("Description")` stored the + * double quotes in the attribute name. check and exec were clean; mx check said + * [CE1613] "The selected attribute 'ModA.EntA."Description"' no longer exists." + * and describe printed `"Description" as '"Description"'`. + * + * The quotes are identifier quoting, as in `create persistent entity`. After the + * fix describe prints `Description as 'Description'`. + */ + +create module Bug1423; + +create persistent entity Bug1423.EntA ( + Name: string(200), + Description: string(200) +); + +create published odata service Bug1423.Pub ( + path: 'odata/bug1423/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'Bug1423.Ns' +) +{ + publish entity Bug1423.EntA as 'EntAs' (ReadMode: source) + expose (Name as 'Name' (KEY), "Description"); +}; diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index 65df93561..afe50ffc9 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -470,7 +470,7 @@ func parsePublishMicroflowBlock(ctx parser.IPublishMicroflowBlockContext) *ast.P if member.IdentifierOrKeyword() == nil { continue } - p := &ast.PublishedParamDef{Name: member.IdentifierOrKeyword().GetText()} + p := &ast.PublishedParamDef{Name: identifierOrKeywordText(member.IdentifierOrKeyword())} if sl := member.STRING_LITERAL(); sl != nil { p.ExposedName = unquoteStringLit(sl) } @@ -511,7 +511,7 @@ func parseExposeMembers(ctx parser.IExposeClauseContext) []*ast.PublishedMemberD } m := &ast.PublishedMemberDef{ - Name: member.IdentifierOrKeyword().GetText(), + Name: identifierOrKeywordText(member.IdentifierOrKeyword()), } // Optional AS 'ExposedName' diff --git a/mdl/visitor/visitor_odata_quoted_member_test.go b/mdl/visitor/visitor_odata_quoted_member_test.go new file mode 100644 index 000000000..f73c4c977 --- /dev/null +++ b/mdl/visitor/visitor_odata_quoted_member_test.go @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import "testing" + +// mendixlabs/mxcli#1423: `expose (..., "Description")` stored the double quotes +// as part of the member name, so mx check reported +// CE1613 "The selected attribute 'ModA.EntA."Description"' no longer exists". +// The quotes are identifier-quoting syntax, as in `create persistent entity`. +func TestPublishEntity_QuotedExposeMemberIsUnquoted(t *testing.T) { + svc := odataActionService(t, + " publish entity T.EntA as 'EntAs' (ReadMode: source)\n"+ + " expose (Name as 'Name' (KEY), \"Description\");") + if len(svc.Entities) != 1 || len(svc.Entities[0].Members) != 2 { + t.Fatalf("expected 1 entity with 2 members, got %+v", svc.Entities) + } + if got := svc.Entities[0].Members[1].Name; got != "Description" { + t.Errorf("member name = %q, want %q", got, "Description") + } +} + +// Same grammar rule, published microflow parameters. +func TestPublishMicroflow_QuotedExposeParamIsUnquoted(t *testing.T) { + svc := odataActionService(t, + " publish microflow T.DoThing expose ( \"Description\" as 'description' );") + if len(svc.Microflows) != 1 || len(svc.Microflows[0].Parameters) != 1 { + t.Fatalf("expected 1 microflow with 1 parameter, got %+v", svc.Microflows) + } + if got := svc.Microflows[0].Parameters[0].Name; got != "Description" { + t.Errorf("parameter name = %q, want %q", got, "Description") + } +} From 1da267ccb9b17bf5ec71b61392e257edbc7732c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:57:51 +0000 Subject: [PATCH 10/21] fix: add describe --all and make the module help truthful (mendixlabs/mxcli#1435) Reported: `mxcli describe module Banking -p .mpr` prints only `mdl 1; create or modify module Banking; create or modify module role ...` although the help says "Describe a module (all contents)"; the full emitter needs `-c "describe module Banking with all"`, which is documented only in the changelog. Choice: add an opt-in `--all` flag (DESCRIBE MODULE ... WITH ALL) and fix the help, rather than appending WITH ALL to every describe module. - A plain describe module matches what `-c "describe module X"` prints, so the subcommand and the MDL statement keep one meaning. - Scripts that describe a module for its header and roles keep working. - WITH ALL has open gaps: #1421 (omits document types) and #1425 (truncates at 10k lines without saying so). Making it the default would swap a wrong help line for output that is silently incomplete. `--all` on any other type is refused rather than ignored. Test: TestDescribeModule_HelpMatchesOutput (cmd/mxcli). Control: with the pre-fix help and a pass-through stub it fails with `describe --help promises "all contents" for a plain describe module` and `describe has no --all flag`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...ibe-module-help-promises-all-contents.json | 1 + cmd/mxcli/cmd_describe.go | 23 ++++++++++- cmd/mxcli/cmd_describe_test.go | 39 +++++++++++++++++++ cmd/mxcli/main.go | 1 + 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json new file mode 100644 index 000000000..5ccc584d8 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-describe-module-help-promises-all-contents.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli describe module Banking -p app.mpr` prints only `create or modify module Banking;` and its module roles, although `describe --help` says \"module Describe a module (all contents)\"; the full output needs `-c \"describe module Banking with all\"`, documented only in the changelog", "cause": "describeMDLCommand maps MODULE to a plain DESCRIBE MODULE (module + roles, by design of the MDL statement); the help line described WITH ALL, which the subcommand had no way to request", "file": "cmd/mxcli/cmd_describe.go, cmd/mxcli/main.go", "fix": "Added `--all` (applyDescribeAll appends WITH ALL; refused on any other type) and made the help say what plain describe module prints", "insight": "Fixed the help, not the default: a plain describe module matches `-c \"describe module X\"` and existing scripts that describe a module to get its header, and WITH ALL has known open gaps (#1421 omits doc types, #1425 truncates at 10k lines silently), so silently switching the default would trade a wrong help line for silently incomplete output. When a subcommand's help and the MDL statement disagree, check which one the -c form agrees with before choosing which to change.", "refs": ["mendixlabs/mxcli#1435"]} diff --git a/cmd/mxcli/cmd_describe.go b/cmd/mxcli/cmd_describe.go index 70cde93b9..b825a4861 100644 --- a/cmd/mxcli/cmd_describe.go +++ b/cmd/mxcli/cmd_describe.go @@ -31,7 +31,7 @@ can match both an entity and a document, for example) or for forms that have no single qualified name (module, settings, navigation, module role). Types: - module Describe a module (all contents) + module Describe a module and its module roles (--all: everything in it) entity Describe an entity externalentity Describe an external entity (alias for entity) association Describe an association @@ -78,6 +78,7 @@ Example: mxcli describe -p app.mpr MyModule.Customer # type auto-detected mxcli describe -p app.mpr MyModule.ProcessOrder # type auto-detected mxcli describe -p app.mpr module MyModule + mxcli describe -p app.mpr --all module MyModule # DESCRIBE MODULE ... WITH ALL mxcli describe -p app.mpr entity MyModule.Customer mxcli describe -p app.mpr microflow MyModule.ProcessOrder mxcli describe -p app.mpr --mdl 0 microflow MyModule.ProcessOrder @@ -137,6 +138,12 @@ Example: fmt.Fprintln(os.Stderr, "Multi-word types also accepted: json structure, import mapping, export mapping, consumed rest service, consumed odata service, published odata service, published rest service, task queue, app security, image collection, scheduled event, regular expression, business event service, ai model, knowledge base, consumed mcp service, data transformer, etc.") os.Exit(1) } + all, _ := cmd.Flags().GetBool("all") + mdlCmd, err := applyDescribeAll(objectType, mdlCmd, all) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } exec, logger := newLoggedExecutor("subcommand") defer logger.Close() @@ -339,6 +346,20 @@ func describeMDLCommand(objectType, name string) (mdlCmd string, ok bool) { return "", false } +// applyDescribeAll applies the --all flag: for a module it describes everything +// in it (DESCRIBE MODULE ... WITH ALL), where a plain describe module prints the +// module and its roles only (mendixlabs/mxcli#1435). --all on any other type is +// an error rather than a flag silently ignored. +func applyDescribeAll(objectType, mdlCmd string, all bool) (string, error) { + if !all { + return mdlCmd, nil + } + if objectType != "MODULE" { + return "", fmt.Errorf("--all applies to describe module only") + } + return mdlCmd + " WITH ALL", nil +} + // objectTypeToDescribe maps a catalog `objects` view ObjectType to the `describe` // keyword. Only directly describable types with a `Module.Name` shape are listed. var objectTypeToDescribe = map[string]string{ diff --git a/cmd/mxcli/cmd_describe_test.go b/cmd/mxcli/cmd_describe_test.go index 67131716a..2d07ba4e0 100644 --- a/cmd/mxcli/cmd_describe_test.go +++ b/cmd/mxcli/cmd_describe_test.go @@ -187,3 +187,42 @@ func TestDescribeHelp_StudioProDocumentNames(t *testing.T) { t.Errorf("unitTypeToDescribe[Queues$Queue] = %q, want taskqueue", got) } } + +// TestDescribeModule_HelpMatchesOutput is mendixlabs/mxcli#1435: the help said +// "module Describe a module (all contents)" while the subcommand ran a plain +// DESCRIBE MODULE, which prints only the module and its roles. The contents are +// opt-in with --all (DESCRIBE MODULE ... WITH ALL), and the help says so. +func TestDescribeModule_HelpMatchesOutput(t *testing.T) { + if strings.Contains(describeCmd.Long, "Describe a module (all contents)") { + t.Error(`describe --help promises "all contents" for a plain describe module`) + } + if describeCmd.Flags().Lookup("all") == nil { + t.Fatal("describe has no --all flag") + } + + plain, _ := describeMDLCommand("MODULE", "M") + for _, tc := range []struct { + all bool + want string + }{ + {false, "DESCRIBE MODULE M"}, + {true, "DESCRIBE MODULE M WITH ALL"}, + } { + got, err := applyDescribeAll("MODULE", plain, tc.all) + if err != nil || got != tc.want { + t.Errorf("describe module M (all=%v) = %q, %v; want %q", tc.all, got, err, tc.want) + } + if _, errs := visitor.Build(got); len(errs) > 0 { + t.Errorf("%q does not parse: %v", got, errs) + } + } + + // --all on any other type is refused rather than silently ignored. + entity, _ := describeMDLCommand("ENTITY", "M.E") + if _, err := applyDescribeAll("ENTITY", entity, true); err == nil { + t.Error("describe --all entity M.E: want an error, got none") + } + if got, err := applyDescribeAll("ENTITY", entity, false); err != nil || got != entity { + t.Errorf("describe entity M.E without --all = %q, %v; want %q", got, err, entity) + } +} diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index 78479a7f1..74d08c36e 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -366,6 +366,7 @@ func init() { // Describe command flags describeCmd.Flags().StringP("format", "f", "mdl", "Output format: mdl, json, mermaid, elk, svg") + describeCmd.Flags().Bool("all", false, "With module: describe everything in the module (DESCRIBE MODULE ... WITH ALL), not only the module and its roles") // Search command flags searchCmd.Flags().StringP("format", "f", "table", "Output format: table, names (for JSON use --json; \"json\" is accepted as a deprecated alias)") From d1776cc0561d3f16aa91da8e6faf8291d2945f05 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 18:11:46 +0000 Subject: [PATCH 11/21] fix: resolve System Java actions by name (mendixlabs/mxcli#1445) `list java actions in System` listed System.VerifyPassword, but describe java action System.VerifyPassword Error: java action not found: System.VerifyPassword and exec of a microflow calling it logged warning: could not look up Java action System.VerifyPassword: java action not found: System.VerifyPassword (entity type params will be empty) ReadJavaActionByName only scanned stored units, and resolved the module first, which fails for System (no stored module unit). The System built-ins are virtual and were synthesized on the list paths only. Fall back to meta.BuildSystemJavaActionsFull by name when the module is System and no stored unit matches; stored units still win, unknown System.X stays not found. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...2026-10-12-system-java-action-by-name.json | 1 + .../1445-system-java-action-lookup.mdl | 13 +++++++++ mdl/backend/modelsdk/java_read.go | 23 ++++++++++++++++ mdl/backend/modelsdk/java_system_test.go | 27 +++++++++++++++++++ 4 files changed, 64 insertions(+) create mode 100644 .claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json create mode 100644 mdl-examples/bug-tests/1445-system-java-action-lookup.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json new file mode 100644 index 000000000..e4a815262 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-system-java-action-by-name.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1445"], "symptom": "`list java actions in System` showed System.VerifyPassword, but `describe java action System.VerifyPassword` failed with \"java action not found: System.VerifyPassword\", and exec of a microflow calling it logged \"warning: could not look up Java action System.VerifyPassword: java action not found ... (entity type params will be empty)\"", "cause": "ReadJavaActionByName only scanned stored JavaAction units, after first resolving the module by name, which fails for System since it has no stored module unit; the System built-ins are virtual (meta.SystemJavaActions) and were synthesized on the list paths only", "file": "`mdl/backend/modelsdk/java_read.go` (ReadJavaActionByName, systemJavaActionByName)", "insight": "Virtual System-module members have to be synthesized on EVERY read path of a doctype, not just the list path: when one is fixed, grep the backend for the matching by-name reader. The fallback must also sit before the module lookup, because GetModuleByName(\"System\") returns nothing. Stored units still win, and an unknown System.X stays 'not found' (the test's control)."} diff --git a/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl b/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl new file mode 100644 index 000000000..144bd9235 --- /dev/null +++ b/mdl-examples/bug-tests/1445-system-java-action-lookup.mdl @@ -0,0 +1,13 @@ +mdl 1; +-- mendixlabs/mxcli#1445: System Java actions are platform built-ins with no +-- stored unit. `list java actions` showed them, but describe and the microflow +-- builder's lookup said "java action not found", and exec warned +-- "could not look up Java action System.VerifyPassword ... (entity type params +-- will be empty)". Expected: exec creates the microflow with no lookup warning, +-- and `describe java action System.VerifyPassword` prints its signature. +create or modify microflow MyFirstModule.MF_Bug1445_VerifyPassword () +returns Boolean as $Ok +begin + $Ok = call java action System.VerifyPassword(userName = 'a', password = 'b'); + return $Ok; +end; diff --git a/mdl/backend/modelsdk/java_read.go b/mdl/backend/modelsdk/java_read.go index 8ef40a390..21fd0a8b9 100644 --- a/mdl/backend/modelsdk/java_read.go +++ b/mdl/backend/modelsdk/java_read.go @@ -29,6 +29,9 @@ func (b *Backend) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaA moduleName, actionName := qualifiedName[:dot], qualifiedName[dot+1:] mod, err := b.GetModuleByName(moduleName) if err != nil || mod == nil { + if ja := systemJavaActionByName(moduleName, actionName); ja != nil { + return ja, nil + } return nil, fmt.Errorf("java action not found: %s", qualifiedName) } containers := b.containerSetForModule(string(mod.ID)) // module + nested folders @@ -41,9 +44,29 @@ func (b *Backend) ReadJavaActionByName(qualifiedName string) (*javaactions.JavaA return javaActionFromGen(u.Element, u.ContainerID), nil } } + if ja := systemJavaActionByName(moduleName, actionName); ja != nil { + return ja, nil + } return nil, fmt.Errorf("java action not found: %s", qualifiedName) } +// systemJavaActionByName resolves a System-module platform built-in (e.g. +// System.VerifyPassword), which has no stored unit. The list paths already +// synthesize these, so the by-name read must too or a listed action cannot be +// described or called (mendixlabs/mxcli#1445). Stored units win; this is only +// the fallback. +func systemJavaActionByName(moduleName, actionName string) *javaactions.JavaAction { + if moduleName != "System" { + return nil + } + for _, ja := range meta.BuildSystemJavaActionsFull() { + if ja.Name == actionName { + return ja + } + } + return nil +} + // ListJavaActionsFull reads every Java action unit into the fully-parsed semantic // form (parameters, types, return type) — what the catalog's java-actions builder // and SHOW JAVA ACTIONS need. (The virtual System-module java actions that the diff --git a/mdl/backend/modelsdk/java_system_test.go b/mdl/backend/modelsdk/java_system_test.go index b3855c7c9..695c648c9 100644 --- a/mdl/backend/modelsdk/java_system_test.go +++ b/mdl/backend/modelsdk/java_system_test.go @@ -100,3 +100,30 @@ func TestListJavaActions_StillReturnsStoredActions(t *testing.T) { "the stored ones were dropped", len(got)) } } + +// mendixlabs/mxcli#1445: `list java actions` showed System.VerifyPassword, but +// `describe java action System.VerifyPassword` and the microflow builder's lookup +// both said "java action not found" — the by-name read only scanned stored units. +func TestReadJavaActionByName_ResolvesSystemBuiltIns(t *testing.T) { + b := New() + if err := b.Connect(copyFixture(t)); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + for _, def := range meta.SystemJavaActions { + ja, err := b.ReadJavaActionByName("System." + def.Name) + if err != nil { + t.Errorf("ReadJavaActionByName(System.%s): %v", def.Name, err) + continue + } + if ja.Name != def.Name || len(ja.Parameters) != len(def.Parameters) { + t.Errorf("System.%s: got name %q with %d parameters, want %d", + def.Name, ja.Name, len(ja.Parameters), len(def.Parameters)) + } + } + // Control: an unknown System name must still be "not found". + if _, err := b.ReadJavaActionByName("System.NoSuchAction"); err == nil { + t.Error("System.NoSuchAction resolved; want not found") + } +} From 9d37d28454e1f0696f21800e0b84444870868cf4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:02:31 +0000 Subject: [PATCH 12/21] fix: run --local picks JDK 17 for a Mendix 9 project (mendixlabs/mxcli#1437) Reported: `mxcli run --local --db-type hsqldb` against a Mendix 9.24 app, with JAVA_HOME pointing at JDK 17, fails in mxbuild's Gradle 7.6 with "Unsupported class file major version 65" -- mxcli resolved JDK 21 even though JAVA_HOME is 17, and a 9.x model has no JavaVersion setting to steer the choice. ProjectJavaMajor fell back to DefaultJavaMajor (21) whenever the model stores no JavaVersion, which is always the case on 9.x, so resolveJDK skipped the JDK 17 JAVA_HOME. Derive the fallback from the project's Mendix version instead: 9.x -> 17 (9.x supports Java 11 and 17; 9.24 LTS uses 17), 10.x/11.x -> 21 as before. Also stop the JDK-not-found error suggesting `alter settings runtime ( JavaVersion: '21' )` below the default: on 9.x that statement changes nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...2-run-local-mendix9-defaults-to-jdk21.json | 1 + cmd/mxcli/docker/detect.go | 9 +-- cmd/mxcli/docker/javaversion.go | 32 ++++++++-- cmd/mxcli/docker/javaversion_test.go | 58 +++++++++++++++++++ 4 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json new file mode 100644 index 000000000..00a321c88 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-12-run-local-mendix9-defaults-to-jdk21.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-12", "symptom": "`mxcli run --local --db-type hsqldb` on a Mendix 9.24 app failed in mxbuild's Gradle 7.6 with \"Unsupported class file major version 65\": mxcli resolved JDK 21 and skipped a JAVA_HOME pointing at JDK 17, and the JDK-not-found hint suggested `alter settings runtime ( JavaVersion: '21' )`, which no-ops on 9.x", "cause": "`ProjectJavaMajor` fell back to a flat `DefaultJavaMajor = 21` whenever the model stores no JavaVersion — but a Mendix 9 model never stores one (the setting does not exist before 10.x), and 9.x supports Java 11/17 only. The detect.go hint fired for any major != 21, including 9.x's 17", "file": "`cmd/mxcli/docker/javaversion.go` (`ProjectJavaMajor`, `defaultJavaMajorForMendix`), `cmd/mxcli/docker/detect.go` (`resolveJDKWith` hint gate)", "insight": "\"Setting absent\" meant \"use the platform default\" only while every supported version shared one default; once a version range without the setting has a different JDK, the fallback has to be keyed on the Mendix version, which the reader already has (`ProjectVersion().MajorVersion`). The committed 9.24.30 fixture (modelsdk/mpr/testdata/v1-project) made this testable without mxbuild. Revert proof: ProjectJavaMajor(9.24 fixture) = (21, false), want (17, false)", "refs": ["mendixlabs/mxcli#1437"]} diff --git a/cmd/mxcli/docker/detect.go b/cmd/mxcli/docker/detect.go index ce2654f3c..6a8ed7f6a 100644 --- a/cmd/mxcli/docker/detect.go +++ b/cmd/mxcli/docker/detect.go @@ -406,10 +406,11 @@ func resolveJDKWith(e jdkEnv, major int) (string, error) { } // Which Java a project needs is a MODEL setting, so a user staring at this // has a second way out that a bare "install a JDK" hides: build for a release - // they already have. Only say so when the requirement is not the default, - // because below 11.14 that is the only supported value and the advice would - // be noise. - if major != DefaultJavaMajor { + // they already have. Only say so when the requirement is above the default: + // below 11.14 the default is the only supported value, and a Mendix 9 + // project (JDK 17) has no JavaVersion setting at all, so the statement would + // change nothing (mendixlabs/mxcli#1437). + if major > DefaultJavaMajor { msg += fmt.Sprintf("\n This is the project's own Settings > Model > JavaVersion. To build for a"+ "\n release you already have instead: mxcli -p .mpr -c \"alter settings runtime ( JavaVersion: '%d' )\"", DefaultJavaMajor) } diff --git a/cmd/mxcli/docker/javaversion.go b/cmd/mxcli/docker/javaversion.go index bf8de4394..a4eb9b14a 100644 --- a/cmd/mxcli/docker/javaversion.go +++ b/cmd/mxcli/docker/javaversion.go @@ -8,11 +8,26 @@ import ( "strings" ) -// DefaultJavaMajor is the Java release assumed when a project does not say which -// one it wants, or cannot be read. Every Mendix 9/10/11 project up to 11.13 uses -// it, so it is also the answer for the overwhelming majority of projects. +// DefaultJavaMajor is the Java release assumed when a project cannot be read, or +// when a Mendix 10/11 project does not say which one it wants. Every Mendix 10/11 +// project up to 11.13 uses it. Mendix 9 is the exception: see mendix9JavaMajor. const DefaultJavaMajor = 21 +// mendix9JavaMajor is the JDK for a Mendix 9 project. 9.x supports Java 11 and +// 17 only (9.24 LTS: 17) and its model has no JavaVersion setting, so the +// release cannot be read and 21 is wrong: mxbuild 9.24's Gradle 7.6 rejects it +// with "Unsupported class file major version 65" (mendixlabs/mxcli#1437). +const mendix9JavaMajor = 17 + +// defaultJavaMajorForMendix is the JDK assumed for a project of the given Mendix +// major version that stores no JavaVersion. Zero (unknown) keeps DefaultJavaMajor. +func defaultJavaMajorForMendix(mendixMajor int) int { + if mendixMajor == 9 { + return mendix9JavaMajor + } + return DefaultJavaMajor +} + // ProjectJavaMajor reads the Java release a project is built for. // // Mendix 11.14 is the first version whose blank app asks for something other @@ -26,8 +41,9 @@ const DefaultJavaMajor = 21 // JavaMajorVersion "21" from 11.12 — which settingsoverlay already normalises, // so both spellings arrive here as a bare number. // -// A project that cannot be read, or whose value is not a number, reports -// DefaultJavaMajor and false. The caller then behaves exactly as mxcli did +// A project that stores no numeric value reports the default for its Mendix +// version (defaultJavaMajorForMendix) and false; one that cannot be read +// reports DefaultJavaMajor and false. The caller then behaves exactly as mxcli did // before: guessing a different JDK from an unreadable model would trade a clear // failure for a confusing one. func ProjectJavaMajor(projectPath string) (int, bool) { @@ -43,7 +59,11 @@ func ProjectJavaMajor(projectPath string) (int, bool) { } n, ok := ParseJavaMajor(ps.Model.JavaVersion) if !ok { - return DefaultJavaMajor, false + mendixMajor := 0 + if pv := reader.ProjectVersion(); pv != nil { + mendixMajor = pv.MajorVersion + } + return defaultJavaMajorForMendix(mendixMajor), false } return n, true } diff --git a/cmd/mxcli/docker/javaversion_test.go b/cmd/mxcli/docker/javaversion_test.go index e6d55763c..3953a879e 100644 --- a/cmd/mxcli/docker/javaversion_test.go +++ b/cmd/mxcli/docker/javaversion_test.go @@ -9,6 +9,7 @@ package docker import ( + "errors" "strings" "testing" ) @@ -117,6 +118,63 @@ func TestJavaMajorOrDefault(t *testing.T) { } } +// mendixlabs/mxcli#1437: a Mendix 9 model stores no JavaVersion at all, and the +// fallback was a flat 21 — so `run --local` skipped a JDK 17 JAVA_HOME and +// mxbuild 9.24's Gradle failed with "Unsupported class file major version 65". +// The fallback must come from the project's Mendix version. +func TestProjectJavaMajorDefaultsToJava17ForMendix9(t *testing.T) { + p := v1Fixture(t) // Mendix 9.24.30, no JavaVersion setting + got, fromProject := ProjectJavaMajor(p) + if got != 17 || fromProject { + t.Errorf("ProjectJavaMajor(9.24 project) = (%d, %v), want (17, false)", got, fromProject) + } +} + +func TestDefaultJavaMajorForMendixVersion(t *testing.T) { + tests := []struct { + major int + want int + }{ + {9, 17}, + {10, DefaultJavaMajor}, + {11, DefaultJavaMajor}, + {0, DefaultJavaMajor}, // unknown version: behave as before + } + for _, tc := range tests { + if got := defaultJavaMajorForMendix(tc.major); got != tc.want { + t.Errorf("defaultJavaMajorForMendix(%d) = %d, want %d", tc.major, got, tc.want) + } + } +} + +// The "build for a release you already have" hint suggests lowering JavaVersion +// to 21. For a Mendix 9 project (needs 17) that advice is a no-op: 9.x has no +// JavaVersion setting, and 21 is not a JDK it can build with. +func TestResolveJDKDoesNotSuggestAlterSettingsBelowDefault(t *testing.T) { + env := jdkEnv{ + goos: "linux", + getenv: func(string) string { return "" }, + lookPath: func(string) (string, error) { return "", errors.New("not found") }, + evalSymlinks: func(p string) (string, error) { return p, nil }, + isJDK: func(string, int) bool { return false }, + glob: func(string) ([]string, error) { return nil, nil }, + searchPaths: func(int) []string { return nil }, + versionOf: func(string) string { return "" }, + } + _, err := resolveJDKWith(env, 17) + if err == nil { + t.Fatal("expected JDK 17 not found") + } + if strings.Contains(err.Error(), "alter settings") { + t.Errorf("JDK 17 error suggests alter settings, which no-ops on Mendix 9.x:\n%v", err) + } + // A project asking above the default can still be steered down to it. + _, err = resolveJDKWith(env, 25) + if err == nil || !strings.Contains(err.Error(), "alter settings") { + t.Errorf("JDK 25 error should keep the alter settings hint, got: %v", err) + } +} + func TestProjectJavaMajorFallsBackOnAnUnreadableProject(t *testing.T) { got, fromProject := ProjectJavaMajor("/nonexistent/project.mpr") if got != DefaultJavaMajor || fromProject { From 5a33866db63b16056764585e14310f46cd36a911 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:03:22 +0000 Subject: [PATCH 13/21] fix: describe user role always states CheckSecurity (mendixlabs/mxcli#1427) Reported: "`describe user role` omits `CheckSecurity` when it is `false`", so the replay re-creates the role with the default -- "Silent security change on replay (round-trip drift)". Expected: the output carries `CheckSecurity: false`, as `show user roles` reports it. formatUserRoleMDL printed CheckSecurity only when true, but a new role is written with true (Security$UserRole.CheckSecurity defaults to true), and `create or modify` keeps an unstated stored value, so false was lost both on recreate and on replay onto an existing role. It is now always printed. The executor mock's AddUserRole built roles with Go's zero CheckSecurity, which hid this from the #707 round-trip test; it now mirrors the real writer's default. Test: TestIssue1427_UserRoleCheckSecurityFalseSurvivesReplay fails before the fix ("recreated role has CheckSecurity true, want false", "modified role kept CheckSecurity true, want false"). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...-user-role-drops-check-security-false.json | 1 + .../1427-user-role-check-security-false.mdl | 21 ++++++++++++ mdl/executor/cmd_security.go | 10 +++--- mdl/executor/issue707_remaining_test.go | 34 +++++++++++++++++-- 4 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json create mode 100644 mdl-examples/bug-tests/1427-user-role-check-security-false.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json new file mode 100644 index 000000000..4e041d03c --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-describe-user-role-drops-check-security-false.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "`describe user role` omits `CheckSecurity` when it is false; create role with CheckSecurity: false, describe, drop, replay -> `show user roles` says Check Security: Yes. Silent security change on replay.", "cause": "formatUserRoleMDL followed 'leave out a property at Mendix's default (R12)' but printed CheckSecurity only when true, the inverse of the default: Security$UserRole.CheckSecurity defaults to true (modelsdk/canon/studiopro_property_defaults.json), and genSec.NewUserRole writes it so. `create or modify` keeps an unstated stored value too, so even replay onto an existing role kept true.", "fix": "Always print `CheckSecurity: ` in formatUserRoleMDL.", "insight": "An 'omit at default' describer is only correct if the omitted value equals what the writer gives a new element; check the default in studiopro_property_defaults.json, not Go's zero value. The executor mock hid it: userRoleCtx's AddUserRole built roles with Go's zero CheckSecurity (false), matching the describer's wrong assumption, so the #707 round-trip test was green. A mock write method must mirror the real writer's defaults or the round trip proves nothing. For a security flag prefer always-explicit: create-or-modify keeps unstated values, so omission is never safe on replay onto an existing role.", "issue": "mendixlabs/mxcli#1427", "file": "mdl/executor/cmd_security.go (formatUserRoleMDL)", "test": "mdl/executor/issue707_remaining_test.go TestIssue1427_UserRoleCheckSecurityFalseSurvivesReplay; mdl-examples/bug-tests/1427-user-role-check-security-false.mdl; control: before the fix 'recreated role has CheckSecurity true, want false' and 'modified role kept CheckSecurity true, want false'"} diff --git a/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl b/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl new file mode 100644 index 000000000..a1a4ab84d --- /dev/null +++ b/mdl-examples/bug-tests/1427-user-role-check-security-false.mdl @@ -0,0 +1,21 @@ +mdl 1; +-- mendixlabs/mxcli#1427: `describe user role` omitted `CheckSecurity` when it +-- was false. A new user role is written with CheckSecurity true, so describe -> +-- drop -> replay silently re-enabled security checking. +-- +-- Verify: +-- +-- mxcli exec 1427-user-role-check-security-false.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe user role Bug1427_User" +-- -> output contains `CheckSecurity: false` +-- save that output, `drop user role Bug1427_User;`, exec the saved output +-- mxcli -p app.mpr -c "show user roles" +-- -> Bug1427_User still shows Check Security: No + +create module Bug1427; +create module role Bug1427.RoleA; + +create or modify user role Bug1427_User ( + ModuleRoles: (Bug1427.RoleA), + CheckSecurity: false +); diff --git a/mdl/executor/cmd_security.go b/mdl/executor/cmd_security.go index 0dec8a201..1edb2cee6 100644 --- a/mdl/executor/cmd_security.go +++ b/mdl/executor/cmd_security.go @@ -870,8 +870,7 @@ func describeUserRole(ctx *ExecContext, name ast.QualifiedName) error { // replaces had no slot for the description or the check-security flag, which // describe printed as `--` comments that a replay dropped, and it could not // write a role with no module roles at all. A property at Mendix's default for -// a new role is left out (R12), so an empty role is `create or modify user -// role R;`. +// a new role is left out (R12), except CheckSecurity, which is always stated. func formatUserRoleMDL(ur *security.UserRole) string { var props []string if len(ur.ModuleRoles) > 0 { @@ -889,9 +888,10 @@ func formatUserRoleMDL(ur *security.UserRole) string { if ur.ManageUsersWithoutRoles { props = append(props, "ManageUsersWithoutRoles: true") } - if ur.CheckSecurity { - props = append(props, "CheckSecurity: true") - } + // Always stated: a new role is written with CheckSecurity true, so leaving + // out a false value re-enabled security checking on replay, and `create or + // modify` keeps an unstated stored value either way (#1427). + props = append(props, fmt.Sprintf("CheckSecurity: %t", ur.CheckSecurity)) head := "create or modify user role " + ur.Name switch len(props) { case 0: diff --git a/mdl/executor/issue707_remaining_test.go b/mdl/executor/issue707_remaining_test.go index e47b86e4e..851fbc92a 100644 --- a/mdl/executor/issue707_remaining_test.go +++ b/mdl/executor/issue707_remaining_test.go @@ -48,7 +48,10 @@ func userRoleCtx(t *testing.T, roles ...*security.UserRole) (*ExecContext, *byte IsConnectedFunc: func() bool { return true }, GetProjectSecurityFunc: func() (*security.ProjectSecurity, error) { return ps, nil }, AddUserRoleFunc: func(_ model.ID, name string, mrs []string, manageAll bool) error { - ps.UserRoles = append(ps.UserRoles, &security.UserRole{Name: name, ModuleRoles: mrs, ManageAllRoles: manageAll}) + // CheckSecurity: true is what the real writer gives a new role + // (Security$UserRole.CheckSecurity defaults to true), so a replay + // that does not state it gets true, as on disk (#1427). + ps.UserRoles = append(ps.UserRoles, &security.UserRole{Name: name, ModuleRoles: mrs, ManageAllRoles: manageAll, CheckSecurity: true}) return nil }, AlterUserRoleModuleRolesFunc: func(_ model.ID, name string, add bool, mrs []string) error { @@ -134,11 +137,38 @@ func TestIssue707_UserRoleWithoutModuleRolesReparses(t *testing.T) { assertNoError(t, describeUserRole(ctx, ast.QualifiedName{Name: "Empty"})) out := buf.String() stmt := findStmt[*ast.CreateUserRoleStmt](t, reparse(t, out), out) - if stmt.Name != "Empty" || len(stmt.ModuleRoles) != 0 || stmt.Description != nil || stmt.CheckSecurity != nil { + if stmt.Name != "Empty" || len(stmt.ModuleRoles) != 0 || stmt.Description != nil || + stmt.CheckSecurity == nil || *stmt.CheckSecurity { // stated even at false (#1427) t.Errorf("empty role re-parses as %+v\n%s", stmt, out) } } +// mendixlabs/mxcli#1427: describe printed `CheckSecurity` only when true, but a +// new role is written with true, so a role with security checking off came back +// from describe → drop → replay with it silently re-enabled. Replayed onto an +// existing role with it on, the unstated property kept true as well. +func TestIssue1427_UserRoleCheckSecurityFalseSurvivesReplay(t *testing.T) { + stored := &security.UserRole{Name: "UserA", ModuleRoles: []string{"ModA.RoleA"}, CheckSecurity: false} + src, srcBuf, _ := userRoleCtx(t, stored) + assertNoError(t, describeUserRole(src, ast.QualifiedName{Name: "UserA"})) + out := srcBuf.String() + stmt := findStmt[*ast.CreateUserRoleStmt](t, reparse(t, out), out) + + // Replay onto a project without the role (describe, drop, recreate). + dst, _, ps := userRoleCtx(t) + assertNoError(t, execCreateUserRole(dst, stmt)) + if len(ps.UserRoles) != 1 || ps.UserRoles[0].CheckSecurity { + t.Errorf("recreated role has CheckSecurity %v, want false\n%s", ps.UserRoles[0].CheckSecurity, out) + } + + // Replay onto an existing role that has it on. + mod, _, mps := userRoleCtx(t, &security.UserRole{Name: "UserA", ModuleRoles: []string{"ModA.RoleA"}, CheckSecurity: true}) + assertNoError(t, execCreateUserRole(mod, stmt)) + if mps.UserRoles[0].CheckSecurity { + t.Errorf("modified role kept CheckSecurity true, want false\n%s", out) + } +} + // Control: `create or modify` states a property only when the statement does, // so a replay of the old positional form onto an existing role keeps the // stored description rather than clearing it. From 9e51dd33fa867460813c777c740cae08664d9c7f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:14:20 +0000 Subject: [PATCH 14/21] fix: describe child slots with the definition's keyword (mendixlabs/mxcli#1400) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: describe emits `content { }` for native pluggable-widget child slots (Repeater, SafeAreaView); check then fails with "`content` is not a container of `repeater` — it declares: template [MDL-WIDGET26]" and exec with "content needs a name". The widget definition names a widgets-typed slot with mdlContainerForWidgetSlot (content -> TEMPLATE); describe's extractChildSlots used deriveObjectListKeyword, the object-list convention, which leaves `content` unchanged. Describe now calls the same function as the definition. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...d-slot-content-described-not-template.json | 1 + mdl/executor/cmd_pages_describe_childslots.go | 11 +++- .../cmd_pages_describe_containers_test.go | 51 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json new file mode 100644 index 000000000..d86da20d4 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-child-slot-content-described-not-template.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1400: describe emits `content { }` for native pluggable-widget child slots (Repeater, SafeAreaView); check fails \"`content` is not a container of `repeater` — it declares: template [MDL-WIDGET26]\" and exec fails \"content needs a name\". 35 occurrences in the round-trip runs.", "cause": "Two derivations of one keyword. The widget definition names a widgets-typed slot with mdlContainerForWidgetSlot (content -> TEMPLATE, plus per-widget overrides); describe's extractChildSlots borrowed deriveObjectListKeyword, the OBJECT-LIST convention (singularise, its own override table), which leaves `content` as is.", "file": "mdl/executor/cmd_pages_describe_childslots.go", "fix": "extractChildSlots derives the keyword with mdlContainerForWidgetSlot(widgetID, key), the function the definition side uses.", "test": "TestExtractChildSlots_KeywordMatchesDefinition (tagContentContainer slot is the control)", "insight": "**When describe and the definition each compute a keyword, they must call the same function** — describe borrowed the nearest-looking helper (object lists) and agreed with the definition on every key except the overridden ones, so only a widget with a `content` slot showed it. The MDL-WIDGET26 message lists what the definition declares; compare that against the describe output before reading either side's code. Not covered: a placeholder inside a SafeAreaView is invisible to layoutPlaceholderNames (mdl/backend/modelsdk/layout_write.go), which walks only typed containers, not a CustomWidget's raw child slots."} diff --git a/mdl/executor/cmd_pages_describe_childslots.go b/mdl/executor/cmd_pages_describe_childslots.go index 6cbd03c54..4c64a8761 100644 --- a/mdl/executor/cmd_pages_describe_childslots.go +++ b/mdl/executor/cmd_pages_describe_childslots.go @@ -13,7 +13,8 @@ import ( // items. type rawChildSlot struct { // Keyword is the MDL container keyword, derived from the property key the - // same way an object list's is (tagContentContainer -> tagcontentcontainer). + // same way the widget definition derives it (mdlContainerForWidgetSlot: + // tagContentContainer -> tagcontentcontainer, content -> template). Keyword string // PropertyKey is the stored key, kept so the emitted MDL can be matched back // to the document when debugging. @@ -56,6 +57,8 @@ func extractChildSlots(ctx *ExecContext, w map[string]any, entityContext string) return nil } + widgetID := extractCustomWidgetID(ctx, w) + var out []rawChildSlot for _, prop := range getBsonArrayElements(obj["Properties"]) { propMap, ok := prop.(map[string]any) @@ -87,7 +90,11 @@ func extractChildSlots(ctx *ExecContext, w map[string]any, entityContext string) continue } - kw := strings.ToLower(deriveObjectListKeyword(key)) + // The keyword the widget definition declares for this slot, so exec + // reads the describe back: a `content` slot is `template` there + // (mendixlabs/mxcli#1400). deriveObjectListKeyword is the OBJECT-LIST + // convention and named it `content`. + kw := strings.ToLower(mdlContainerForWidgetSlot(widgetID, key)) if kw == "" { kw = strings.ToLower(key) } diff --git a/mdl/executor/cmd_pages_describe_containers_test.go b/mdl/executor/cmd_pages_describe_containers_test.go index 1cdcd5f88..2a4b55f66 100644 --- a/mdl/executor/cmd_pages_describe_containers_test.go +++ b/mdl/executor/cmd_pages_describe_containers_test.go @@ -145,3 +145,54 @@ func TestExtractChildSlots_NoContainers(t *testing.T) { t.Errorf("got %+v, want none", got) } } + +// A child slot is described under the keyword its widget definition declares +// for it, or exec cannot read the describe back. A native Repeater's (and +// SafeAreaView's) `content` slot is `template` on the definition side +// (mdlContainerForWidgetSlot); describe derived `content` from the bare key, +// and check refused it: "`content` is not a container of `repeater` — it +// declares: template [MDL-WIDGET26]" (mendixlabs/mxcli#1400). The +// tagContentContainer slot is the control: a key without an override is +// described as itself, as before. +func TestExtractChildSlots_KeywordMatchesDefinition(t *testing.T) { + w := map[string]any{ + "Type": map[string]any{ + "WidgetId": "com.mendix.widget.native.repeater.Repeater", + "ObjectType": map[string]any{ + "PropertyTypes": []any{ + int32(3), + map[string]any{"$ID": "t1", "PropertyKey": "content"}, + map[string]any{"$ID": "t2", "PropertyKey": "tagContentContainer"}, + }, + }, + }, + "Object": map[string]any{ + "Properties": []any{ + int32(3), + map[string]any{ + "TypePointer": "t1", + "Value": map[string]any{"Widgets": []any{int32(3), + map[string]any{"$Type": "Forms$DynamicText", "Name": "a"}}}, + }, + map[string]any{ + "TypePointer": "t2", + "Value": map[string]any{"Widgets": []any{int32(3), + map[string]any{"$Type": "Forms$DynamicText", "Name": "b"}}}, + }, + }, + }, + } + + got := map[string]string{} + for _, s := range extractChildSlots(&ExecContext{}, w, "") { + got[s.PropertyKey] = s.Keyword + } + for key, want := range map[string]string{ + "content": "template", + "tagContentContainer": "tagcontentcontainer", + } { + if got[key] != want { + t.Errorf("slot %q described as %q, want %q (the keyword the definition declares)", key, got[key], want) + } + } +} From 68022b6cda5ca68e0e241d8087e6d3640c6a9378 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:21:08 +0000 Subject: [PATCH 15/21] fix: resolve image refs against collections the script creates (mendixlabs/mxcli#1419) Reported: a script that creates image collection ModA.Icons and then a page with `image img1 (Image: 'ModA.Icons.pix')` fails check with image collection not found: ModA.Icons (referenced as ModA.Icons.pix) and exec with "Refusing to execute: 1 unresolved reference(s) above. Nothing was written." Split into two scripts, it passes. validateWidgetReferences resolved image references against the project only, under a stale comment saying MDL cannot create an image collection. scriptContext now records the collections and images that CreateImageCollectionStmt declares, and they are merged into the project sets (only when those could be read, so a failed read still reports nothing). An undeclared image in a script-created collection is still reported. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...0-12-image-collection-same-script-ref.json | 1 + .../1419-image-collection-same-script-ref.mdl | 26 ++++++++ mdl/executor/helpers.go | 14 +++- mdl/executor/validate.go | 18 +++++ .../validate_widget_image_ref_script_test.go | 65 +++++++++++++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json create mode 100644 mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl create mode 100644 mdl/executor/validate_widget_image_ref_script_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json new file mode 100644 index 000000000..3ddccd1c4 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-image-collection-same-script-ref.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-12","symptom":"a page whose `image` widget names `ModA.Icons.pix` was refused by check (\"image collection not found: ModA.Icons (referenced as ModA.Icons.pix)\") and by exec (\"Refusing to execute: 1 unresolved reference(s) above. Nothing was written.\") when the same script created the collection; split into two scripts it passed (mendixlabs/mxcli#1419)","cause":"validateWidgetReferences resolved image refs against the project only, under a comment saying MDL cannot create an image collection — stale since CreateImageCollectionStmt existed; scriptContext had no image-collection or image sets","file":"`mdl/executor/helpers.go` (validateWidgetReferences image branch), `mdl/executor/validate.go` (scriptContext.imageCollections/images, collectSingle)","insight":"**A comment justifying a missing same-script exemption ages into the bug.** Every other widget reference kind merges `sc.` into the project set; grep `validateWidgetReferences` for a kind that does not, and check whether the statement that creates it now exists. Merge only into non-nil project sets: nil means the project could not be read and must keep reporting nothing. The pre-check context is whole-script (collectDefinitions), like the other kinds — statement-order is not enforced here","refs":["mendixlabs/mxcli#1419"],"ce":[]} diff --git a/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl b/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl new file mode 100644 index 000000000..55abfd73b --- /dev/null +++ b/mdl-examples/bug-tests/1419-image-collection-same-script-ref.mdl @@ -0,0 +1,26 @@ +mdl 1; + +/** + * mendixlabs/mxcli#1419: exec's reference pre-check did not see an image + * collection created earlier in the same script. check reported + * image collection not found: ModA.Icons (referenced as ModA.Icons.pix) + * and exec refused: "Refusing to execute: 1 unresolved reference(s) above. + * Nothing was written." Split into two scripts, the same statements passed. + * + * The image file path is resolved from the current working directory, as in + * doctype-tests/19-image-collection-examples.mdl. + */ + +create module Bug1419; + +create image collection Bug1419.Icons { + image pix ( File: '../../docs/images/mxcli-cli.png' ) +}; + +create page Bug1419.IconPage ( + Title: 'Icon page', Layout: Atlas_Core.Atlas_Default +) { + placeholder Main { + image img1 (Image: 'Bug1419.Icons.pix') + } +}; diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index 4e984d8c6..b7ef55e84 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -288,9 +288,19 @@ func validateWidgetReferences(ctx *ExecContext, widgets []*ast.WidgetV3, sc *scr } if len(refs.images) > 0 { - // No same-script exemption: MDL cannot create an image collection entry, - // so an image reference can only ever resolve against the project. + // The project's images plus the ones this script creates + // (mendixlabs/mxcli#1419). A nil set means the project could not be read, + // and stays nil: imageRefErrors then reports nothing rather than calling + // every other collection missing. known, collections := buildImageQualifiedNames(ctx) + if known != nil && collections != nil { + for qn := range sc.imageCollections { + collections[qn] = true + } + for qn := range sc.images { + known[qn] = true + } + } errors = append(errors, imageRefErrors(refs.images, known, collections)...) } diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 5f7ba7923..2e9ef0c3a 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -51,6 +51,14 @@ type scriptContext struct { // (mendixlabs/mxcli#1211). queues map[string]bool + // Image collections created (Module.Collection) and the images they declare + // (Module.Collection.Image), both lower-cased like buildImageQualifiedNames. + // Missing, an image widget naming a collection the same script creates was + // "image collection not found" and exec refused the script + // (mendixlabs/mxcli#1419). + imageCollections map[string]bool + images map[string]bool + // Microflow-typed parameters of the Java actions the script creates // (Module.Action -> parameter name). Such a parameter takes a microflow // NAME, never an expression (mendixlabs/mxcli#1210). @@ -133,6 +141,8 @@ func newScriptContext() *scriptContext { menus: make(map[string]bool), constants: make(map[string]bool), queues: make(map[string]bool), + imageCollections: make(map[string]bool), + images: make(map[string]bool), javaActionFlowParams: make(map[string]map[string]bool), entityDecls: make(map[string]*ast.CreateEntityStmt), @@ -292,6 +302,14 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { if s.Name.Module != "" { sc.menus[s.Name.String()] = true } + case *ast.CreateImageCollectionStmt: + if s.Name.Module != "" { + qn := strings.ToLower(s.Name.String()) + sc.imageCollections[qn] = true + for _, img := range s.Images { + sc.images[qn+"."+strings.ToLower(img.Name)] = true + } + } case *ast.CreateWorkflowStmt: if s.Name.Module != "" { sc.workflows[s.Name.String()] = true diff --git a/mdl/executor/validate_widget_image_ref_script_test.go b/mdl/executor/validate_widget_image_ref_script_test.go new file mode 100644 index 000000000..0fa42568d --- /dev/null +++ b/mdl/executor/validate_widget_image_ref_script_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1419: an image collection created earlier in the same script +// was invisible to the reference pre-check, so +// +// create image collection ModA.Icons (image pix from file '…'); +// create page … { image img1 (Image: 'ModA.Icons.pix') } +// +// was refused by check with +// +// image collection not found: ModA.Icons (referenced as ModA.Icons.pix) +// +// and by exec with "Refusing to execute: 1 unresolved reference(s) above". +// Split into two scripts, the same statements passed. + +func scriptWithImageCollection(module, name string, images ...string) *scriptContext { + stmt := &ast.CreateImageCollectionStmt{Name: ast.QualifiedName{Module: module, Name: name}} + for _, img := range images { + stmt.Images = append(stmt.Images, ast.ImageItem{Name: img}) + } + sc := newScriptContext() + sc.collectSingle(stmt) + return sc +} + +func TestWidgetRefs_ImageInScriptCreatedCollectionIsResolved(t *testing.T) { + ctx := imageCollectionCtx(t) + sc := scriptWithImageCollection("ModA", "Icons", "pix") + + errs := validateWidgetReferences(ctx, imageRefWidget("image", "Image", "ModA.Icons.pix"), sc) + if len(errs) != 0 { + t.Fatalf("an image in a collection the script creates was refused: %v", errs) + } + // Mendix resolves names case-insensitively; the script's spelling must too. + errs = validateWidgetReferences(ctx, imageRefWidget("staticimage", "Image", "moda.ICONS.Pix"), sc) + if len(errs) != 0 { + t.Fatalf("a case difference against a script-created image was refused: %v", errs) + } + // The project's own collections still resolve alongside the script's. + errs = validateWidgetReferences(ctx, imageRefWidget("image", "Image", "MyFirstModule.Images.gallery"), sc) + if len(errs) != 0 { + t.Fatalf("a project image was refused once the script defined a collection: %v", errs) + } +} + +// CONTROL: the script's collection is a real answer, not an exemption — an +// image it does not declare is still reported. +func TestWidgetRefs_MissingImageInScriptCreatedCollectionIsReported(t *testing.T) { + ctx := imageCollectionCtx(t) + sc := scriptWithImageCollection("ModA", "Icons", "pix") + + errs := validateWidgetReferences(ctx, imageRefWidget("image", "Image", "ModA.Icons.nope"), sc) + if len(errs) != 1 || !strings.Contains(errs[0], "image not found: nope") { + t.Fatalf("an image the script's collection does not hold was accepted; got %v", errs) + } +} From 3d5f0d714a718fcde26190537729b68053b99a1d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:24:57 +0000 Subject: [PATCH 16/21] fix: qualify a list sort's inherited attribute with its declaring entity (mendixlabs/mxcli#1430) Reported: `$L = sort $BookChunkList by Similarity desc;`, where Similarity is declared on the generalization, fails in mxbuild with CE1613 "The selected attribute 'Embeddings_SemanticSearch.BookChunk.Similarity' no longer exists." The list-operation sort prefixed a bare attribute with the list's own entity. The retrieve `sort by` path already resolves the declaring entity through resolveAttributeInEntityHierarchy; the list sort now does the same, falling back to the list entity when the model does not resolve the name. FIND/FILTER already qualify through resolveMemberChange, which walks the hierarchy. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...-list-sort-inherited-attribute-ce1613.json | 1 + .../1430-list-sort-inherited-attribute.mdl | 27 +++++++ .../cmd_microflows_builder_actions.go | 10 ++- ...lows_list_sort_inherited_attribute_test.go | 71 +++++++++++++++++++ 4 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json create mode 100644 mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl create mode 100644 mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json new file mode 100644 index 000000000..f788103bc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-list-sort-inherited-attribute-ce1613.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1430: list-operation `$L = sort $BookChunkList by Similarity desc;` with Similarity declared on the generalization wrote `Embeddings_SemanticSearch.BookChunk.Similarity`; mxbuild CE1613 \"The selected attribute ... no longer exists.\" Describe round-trips identically, so a describe-only comparison calls it in sync.", "cause": "addListOperationAction's ListOpSort case qualified a bare attribute as listEntity + \".\" + attr. The retrieve `sort by` path had been fixed for the same class (CapTrackV2 §13) with resolveAttributeInEntityHierarchy, but the list-operation sort was a separate copy of the qualification and was not.", "file": "mdl/executor/cmd_microflows_builder_actions.go", "fix": "ListOpSort qualifies via fb.resolveAttributeInEntityHierarchy, falling back to the list entity when the model does not resolve it (unit tests without a backend, unknown attribute). FIND/FILTER already go through resolveMemberChange, which walks the hierarchy.", "test": "TestListSort_InheritedAttributeUsesTheDeclaringEntity, TestListSort_OwnAttributeIsUnchanged; mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl", "insight": "**When a qualification bug is fixed, grep every `entityType + \".\" +` in the builder — the fix for one path does not reach its copies.** The retrieve sort, list sort and aggregate attribute each built Module.Entity.Attr independently; only the first walked the generalization chain. The aggregate `sum/min/max $L by Attr` path still prefixes blindly (same CE1613 class, not fixed here). Describe cannot detect this: it prints the bare attribute name either way, so only the stored AttributeQualifiedName (bson dump) or mxbuild shows it. The test reuses inheritedSortBackend from the §13 test; written before the fix it failed with stored [SyntheticApp.AppUser.Name]."} diff --git a/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl b/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl new file mode 100644 index 000000000..811a68b75 --- /dev/null +++ b/mdl-examples/bug-tests/1430-list-sort-inherited-attribute.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1430: list-operation sort on an inherited attribute +-- ============================================================================ +-- +-- `$L = sort $BookChunkList by Similarity desc;` where Similarity is declared on +-- the generalization was written as `Module.BookChunk.Similarity` and mxbuild +-- rejected it: +-- +-- CE1613 "The selected attribute '...BookChunk.Similarity' no longer exists." +-- +-- The sort must be qualified with the DECLARING entity (Bug1430.Chunk), as the +-- retrieve `sort by` already is. Expected after exec: describe shows the sort, +-- and `mx check` reports no CE1613. +-- ============================================================================ + +create module Bug1430; + +create persistent entity Bug1430.Chunk ( Similarity: Decimal ); + +create persistent entity Bug1430.BookChunk extends Bug1430.Chunk ( Title: String(100) ); + +create microflow Bug1430.MF_SortInherited ($BookChunkList: list of Bug1430.BookChunk) returns list of Bug1430.BookChunk +begin + $L = sort $BookChunkList by Similarity desc; + return $L; +end; diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 172e6d0e5..2cace1b8a 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -1775,10 +1775,16 @@ func (fb *flowBuilder) addListOperationAction(s *ast.ListOperationStmt) model.ID if !spec.Ascending { direction = microflows.SortDirectionDescending } - // Build fully qualified attribute name: Entity.Attribute + // Qualify with the entity that DECLARES the attribute, as the + // retrieve sort does: an inherited name qualified with the list's + // own entity is CE1613 (mendixlabs/mxcli#1430). attrQN := spec.Attribute if entityType != "" && !strings.Contains(spec.Attribute, ".") { - attrQN = entityType + "." + spec.Attribute + if declared, ok := fb.resolveAttributeInEntityHierarchy(entityType, spec.Attribute); ok { + attrQN = declared + } else { + attrQN = entityType + "." + spec.Attribute + } } if fb.qualifiedMembersOnly { fb.refuseUnqualifiedAttribute(attrQN, spec.Attribute, entityType) diff --git a/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go b/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go new file mode 100644 index 000000000..01841f0e8 --- /dev/null +++ b/mdl/executor/cmd_microflows_list_sort_inherited_attribute_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1430: the list-operation `sort $L by Attr` qualified an +// INHERITED attribute against the list's own entity, which mxbuild rejects: +// +// CE1613 "The selected attribute 'Embeddings_SemanticSearch.BookChunk.Similarity' +// no longer exists." +// +// The retrieve `sort by` path was fixed for the same class (CapTrackV2 §13) via +// resolveAttributeInEntityHierarchy; the list-operation path still prefixed +// the list entity blindly. Reuses inheritedSortBackend: AppUser extends +// SyntheticBase.User, which declares Name; AppUser declares FullName itself. +func listSortAttrOf(t *testing.T, attr string) []string { + t.Helper() + fb := &flowBuilder{ + backend: inheritedSortBackend(), + spacing: 100, + varTypes: map[string]string{"Users": "List of SyntheticApp.AppUser"}, + } + fb.addListOperationAction(&ast.ListOperationStmt{ + OutputVariable: "Sorted", + Operation: ast.ListOpSort, + InputVariable: "Users", + SortSpecs: []ast.SortSpec{{Attribute: attr, Ascending: false}}, + }) + if len(fb.errors) > 0 { + t.Fatalf("unexpected errors: %v", fb.errors) + } + var paths []string + for _, obj := range fb.objects { + act, ok := obj.(*microflows.ActionActivity) + if !ok { + continue + } + lo, ok := act.Action.(*microflows.ListOperationAction) + if !ok { + continue + } + if so, ok := lo.Operation.(*microflows.SortOperation); ok { + for _, s := range so.Sorting { + paths = append(paths, s.AttributeQualifiedName) + } + } + } + return paths +} + +func TestListSort_InheritedAttributeUsesTheDeclaringEntity(t *testing.T) { + paths := listSortAttrOf(t, "Name") + if len(paths) != 1 || paths[0] != "SyntheticBase.User.Name" { + t.Errorf("stored %v, want [SyntheticBase.User.Name] — the list's own entity "+ + "does not declare Name, and mxbuild rejects that reference with CE1613", paths) + } +} + +// CONTROL: an attribute the entity declares itself is still qualified with it. +func TestListSort_OwnAttributeIsUnchanged(t *testing.T) { + paths := listSortAttrOf(t, "FullName") + if len(paths) != 1 || paths[0] != "SyntheticApp.AppUser.FullName" { + t.Errorf("stored %v, want [SyntheticApp.AppUser.FullName]", paths) + } +} From 4a17caea6c65b874378b0c3ad74ec7eeb327e9e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:28:04 +0000 Subject: [PATCH 17/21] =?UTF-8?q?fix:=20explain=20`returns=20=E2=80=A6=20a?= =?UTF-8?q?s=20$X`=20in=20the=20set/declare=20messages=20(mendixlabs/mxcli?= =?UTF-8?q?#1436)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For create or modify microflow M.F () returns Boolean as $Closed begin $Closed = true; return $Closed; end; `$Closed = true;` was refused with "a reassignment says `set` under mdl 1: write `set $Closed = true;`", and `set $Closed = true;` then with "variable 'Closed' is not declared" - each message sent the reporter to the other. The semantics are right and unchanged: `as $X` names the result for callers and declares nothing. Messages only: - the mdl 1 "use set" error also offers `declare $X = …;` for a variable that is new; - the not-declared error on `set $X`, when $X is the flow's `returns … as` name, says the clause does not declare it and shows the declare with the return type. validateFlowBody now receives the return type for this. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...turns-as-variable-undeclared-messages.json | 1 + mdl/executor/cmd_microflows_builder.go | 23 ++++++++++++++++ .../cmd_microflows_builder_actions.go | 4 +-- .../cmd_microflows_builder_validate.go | 13 +++++---- mdl/executor/validate.go | 4 +-- mdl/executor/validate_merge_scope_test.go | 27 +++++++++++++++++++ mdl/visitor/visitor_list_activities.go | 5 ++-- mdl/visitor/visitor_list_activity_test.go | 18 +++++++++++++ 8 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json new file mode 100644 index 000000000..8c4b4d336 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-returns-as-variable-undeclared-messages.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1436"], "symptom": "`create or modify microflow M.F () returns Boolean as $Closed begin $Closed = true; return $Closed; end;` was refused with \"a reassignment says `set` under mdl 1: write `set $Closed = true;`\", and the `set` form then with \"variable 'Closed' is not declared\" (check -p --references) - each message pointed at the other", "cause": "Semantics were right (`returns … as $X` only names the result, Mendix's ReturnVariableName, and declares nothing); the two messages were each written for the common case: the visitor's MDL-V1 setIsMandatory error assumed the target already exists, and the flow validator's not-declared error did not know the flow's return clause", "file": "`mdl/visitor/visitor_list_activities.go` (setIsMandatory message), `mdl/executor/cmd_microflows_builder.go` (addUndeclaredSetError), `mdl/executor/cmd_microflows_builder_validate.go` (validateFlowBody now takes the return type)", "insight": "Message-only fix, no semantics change. The visitor cannot tell a reassignment from a first assignment (it has no scope), so its `set` advice must offer `declare` too. The not-declared error is only reproducible WITH a project: plain `mxcli check` passes the `set` form, because validateFlowBody runs from check --references and exec - reproduce with -p before concluding a report is wrong. validateFlowBody had no return type; flowBuilder.returnType already existed (set by buildFlowGraph on exec) so it is reused, and nothing in the validate path reads it otherwise."} diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 10149a033..89bcc35c2 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -225,6 +225,29 @@ func (fb *flowBuilder) hasDeclaredReturnValue() bool { return fb.returnType != nil && fb.returnType.Type.Kind != ast.TypeVoid } +// addUndeclaredSetError reports `set $X = …` on a variable that is not +// declared. When $X is the flow's `returns … as $X` name the generic message +// reads as contradicting that clause, so it says what the clause does: it names +// the result for callers and declares nothing (mendixlabs/mxcli#1436). +func (fb *flowBuilder) addUndeclaredSetError(target string) { + name := strings.TrimPrefix(target, "$") + if fb.returnType != nil && fb.returnType.Variable != "" && + strings.TrimPrefix(fb.returnType.Variable, "$") == name { + typ := fb.returnType.Type.Kind.String() + if ref := fb.returnType.Type.EntityRef; ref != nil { + typ = ref.Module + "." + ref.Name + } + fb.addErrorWithExample( + fmt.Sprintf("variable '%s' is not declared: `returns … as $%s` names the result for callers "+ + "and does not declare $%s — declare it before the first `set`", name, name, name), + fmt.Sprintf(" declare $%s %s = …;\n ...\n set $%s = …;", name, typ, name)) + return + } + fb.addErrorWithExample( + fmt.Sprintf("variable '%s' is not declared", target), + errorExampleDeclareVariable(target)) +} + // errorExampleDeclareVariable returns an example for declaring a variable. func errorExampleDeclareVariable(varName string) string { // Remove $ prefix if present for cleaner display diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 172e6d0e5..0c7b4be1b 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -67,9 +67,7 @@ func (fb *flowBuilder) addCreateVariableAction(s *ast.DeclareStmt) model.ID { func (fb *flowBuilder) addChangeVariableAction(s *ast.MfSetStmt) model.ID { // Validate that the variable has been declared if !fb.isVariableDeclared(s.Target) { - fb.addErrorWithExample( - fmt.Sprintf("variable '%s' is not declared", s.Target), - errorExampleDeclareVariable(s.Target)) + fb.addUndeclaredSetError(s.Target) } activityX := fb.posX diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index e72773945..179e87322 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -13,13 +13,13 @@ import ( // ValidateMicroflowBody validates the microflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateMicroflowBody(s *ast.CreateMicroflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true, nil) + return validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, nil) } // ValidateNanoflowBody validates the nanoflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true, nil) + return validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, nil) } // validateFlowBody validates parameters and body statements for semantic errors. @@ -27,7 +27,7 @@ func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { // flowBuilder.duplicateNamesOwnedElsewhere. assocs, when check --references has // a project, types an association retrieve as the object or list it yields; // nil leaves every association retrieve a list, as without a project. -func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { +func validateFlowBody(params []ast.MicroflowParam, returns *ast.MicroflowReturnType, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { varTypes := make(map[string]string) declaredVars := make(map[string]string) @@ -57,6 +57,7 @@ func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement fb := &flowBuilder{ varTypes: varTypes, declaredVars: declaredVars, + returnType: returns, // only for messages (addUndeclaredSetError) errors: []string{}, duplicateNamesOwnedElsewhere: duplicatesOwnedElsewhere, checkAssocShapes: assocs, @@ -104,9 +105,7 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { case *ast.MfSetStmt: // Validate that the variable has been declared if !fb.isVariableDeclared(s.Target) { - fb.addErrorWithExample( - fmt.Sprintf("variable '%s' is not declared", s.Target), - errorExampleDeclareVariable(s.Target)) + fb.addUndeclaredSetError(s.Target) } else if fb.assocObjectVars[strings.TrimPrefix(s.Target, "$")] { // Only the association-retrieved object: every other object // producer is visible without a project, and MDL-SET01 reports @@ -466,7 +465,7 @@ func (fb *flowBuilder) validateOutputVariable(varName, statement string) { // counterpart of ValidateMicroflowBody. What a rule may not *contain* is a // separate question, answered by validateRule. func ValidateRuleBody(s *ast.CreateRuleStmt) []string { - return validateFlowBody(s.Parameters, s.Body, false, nil) + return validateFlowBody(s.Parameters, nil, s.Body, false, nil) } // forwardReferenceTarget reports the entity a retrieve over association yields diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 5f7ba7923..6b6257be4 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -711,7 +711,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) + validationErrors := validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside microflow body (pages, microflows, java actions, entities) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if ctx.Connected() { @@ -764,7 +764,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) + validationErrors := validateFlowBody(s.Parameters, s.ReturnType, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside nanoflow body (an excluded nanoflow's are warnings) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if ctx.Connected() { diff --git a/mdl/executor/validate_merge_scope_test.go b/mdl/executor/validate_merge_scope_test.go index 46a1a870f..c789e90aa 100644 --- a/mdl/executor/validate_merge_scope_test.go +++ b/mdl/executor/validate_merge_scope_test.go @@ -45,6 +45,33 @@ func createMicroflowStmtOf(t *testing.T, src string) *ast.CreateMicroflowStmt { return nil } +// mendixlabs/mxcli#1436: `returns Boolean as $Closed` names the result for +// callers and declares nothing, so `set $Closed = true` is correctly refused — +// but "variable 'Closed' is not declared" read as contradicting the very +// clause that names it. The message must say what `as $Closed` does. +func TestValidateFlowBody_SetOnReturnsAsNameExplainsTheClause(t *testing.T) { + stmt := createMicroflowStmtOf(t, "create microflow M.F () returns Boolean as $Closed\nbegin\n set $Closed = true;\n return $Closed;\nend;") + errs := ValidateMicroflowBody(stmt) + if len(errs) != 1 { + t.Fatalf("want exactly one error (semantics unchanged), got %v", errs) + } + for _, want := range []string{"is not declared", "returns … as $Closed", "declare $Closed Boolean"} { + if !strings.Contains(errs[0], want) { + t.Errorf("error should contain %q, got: %s", want, errs[0]) + } + } +} + +// Control: another undeclared variable keeps the generic message, with no +// mention of the `as` clause. +func TestValidateFlowBody_SetOnOtherUndeclaredNameKeepsGenericMessage(t *testing.T) { + stmt := createMicroflowStmtOf(t, "create microflow M.F () returns Boolean as $Closed\nbegin\n set $Other = true;\n return true;\nend;") + errs := ValidateMicroflowBody(stmt) + if len(errs) != 1 || strings.Contains(errs[0], "returns …") { + t.Fatalf("want the generic not-declared error, got %v", errs) + } +} + func TestValidateFlowBody_MergeSeesItsJoinsVariables(t *testing.T) { stmt := createMicroflowStmtOf(t, mergeSectionScopeMDL) if errs := ValidateMicroflowBody(stmt); len(errs) > 0 { diff --git a/mdl/visitor/visitor_list_activities.go b/mdl/visitor/visitor_list_activities.go index 139ec1c3f..98189e43f 100644 --- a/mdl/visitor/visitor_list_activities.go +++ b/mdl/visitor/visitor_list_activities.go @@ -354,9 +354,10 @@ func (b *Builder) ExitSetStatement(ctx *parser.SetStatementContext) { } if ctx.SET() == nil { if b.gate(setIsMandatory, ctx) { - b.addError(fmt.Errorf("line %d: a reassignment says `set` under mdl 1: write `set %s = %s;`. "+ + b.addError(fmt.Errorf("line %d: a reassignment says `set` under mdl 1: write `set %s = %s;`, "+ + "or `declare %s = %s;` if %s is a new variable. "+ "A List operation or Aggregate list activity is written without it, as its own statement "+ - "(`%s = filter $List where …;`)", line, target, valueText, target)) + "(`%s = filter $List where …;`)", line, target, valueText, target, valueText, target, target)) return } b.fixLastNote(setIsMandatory.Code, &ast.Fix{Edits: []ast.TextEdit{insertAt(ctx.GetStart().GetStart(), "set ")}}, "") diff --git a/mdl/visitor/visitor_list_activity_test.go b/mdl/visitor/visitor_list_activity_test.go index 2d1ade9f9..98ea03f89 100644 --- a/mdl/visitor/visitor_list_activity_test.go +++ b/mdl/visitor/visitor_list_activity_test.go @@ -320,6 +320,24 @@ func TestNestedListOperationRefusedUnderMdl1(t *testing.T) { } } +// mendixlabs/mxcli#1436: `$Closed = true;` on a name that was never declared +// (here: one named only by `returns Boolean as $Closed`) was told to write +// `set`, and `set` then failed with "not declared". The message must also +// offer the declaration for a variable that is new. +func TestSetIsMandatory_MessageOffersDeclareForANewVariable(t *testing.T) { + src := "mdl 1;\ncreate or modify microflow M.F () returns Boolean as $Closed\nbegin\n $Closed = true;\n return $Closed;\nend;" + _, errs := Build(src) + if len(errs) == 0 { + t.Fatal("mdl 1 must refuse a reassignment without set") + } + msg := errs[0].Error() + for _, want := range []string{"set $Closed = true;", "declare $Closed = true;"} { + if !strings.Contains(msg, want) { + t.Errorf("error should contain %q, got: %s", want, msg) + } + } +} + // `set` is mandatory for reassignment under mdl 1. func TestSetIsMandatoryUnderMdl1(t *testing.T) { body := "declare $I Integer = 0;\n $I = 5;" From f9945bb02e001a69b3efabc212a32a1b805f8f26 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 19:54:26 +0000 Subject: [PATCH 18/21] test: give the #1423 bug-test key a unique rule so it passes mx check Without it mx check fails CE6624 (an OData key attribute needs a unique validation rule), which hid whether the CE1613 from the quoted name was gone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl index 219963087..a8b209bfe 100644 --- a/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl +++ b/mdl-examples/bug-tests/1423-odata-expose-quoted-member-name.mdl @@ -13,7 +13,7 @@ mdl 1; create module Bug1423; create persistent entity Bug1423.EntA ( - Name: string(200), + Name: string(200) unique error message 'Name must be unique', Description: string(200) ); From 0b464d63e71aac842038cf4dd98eda829c6cf14f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 20:33:06 +0000 Subject: [PATCH 19/21] test: strike 14 JavaScript actions that now round-trip from the allowlists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The separator fix makes these JavaScript actions' describe output parse again, so integration (roundtrip) failed with "no longer breaks parse — strike it from knownFailures … The allowlist may only shrink" for each of them, on both PedApp and TestApp. All 14 entries listed lawParse only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- mdl/roundtrip/allowlist_test.go | 114 +++++++++++------------- mdl/roundtrip/testapp_allowlist_test.go | 14 --- 2 files changed, 50 insertions(+), 78 deletions(-) diff --git a/mdl/roundtrip/allowlist_test.go b/mdl/roundtrip/allowlist_test.go index 728126edb..f83d25a03 100644 --- a/mdl/roundtrip/allowlist_test.go +++ b/mdl/roundtrip/allowlist_test.go @@ -30,70 +30,56 @@ var knownFailures = map[string]knownFailure{ // Plain `create` where `create or modify` is needed (#705 item 5). // #721 G: describe output that does not parse. - "building block Atlas_Web_Content.Alert": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.AlertIcon": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.AlertIcon_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Alert_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Breadcrumb": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Breadcrumb_Underline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_Action": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_ActionWithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Card_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Horizontal_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Form_Vertical_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Heroheader_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_DoubleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_SingleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.ListItem_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.List_Cards": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.List_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Master_Detail": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.PageheaderImage_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Pageheader_WithSearch": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Timeline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Timeline_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Arrow": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Arrow_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Circle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block Atlas_Web_Content.Wizard_Circle_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "building block FeedbackModule.FeedbackWidget": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, - "javascript action DataWidgets.Reset_All_Filters": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action DataWidgets.Reset_Filter": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action DataWidgets.Set_Filter": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.DraftEmail": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.Geocode": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetCurrentLocation": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetCurrentLocationMinimumAccuracy": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.ReverseGeocode": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, - "javascript action NanoflowCommons.SetStorageItemString": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.Share": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.ShowConfirmation": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.ShowProgress": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action NanoflowCommons.SignIn": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, - "javascript action WebActions.TakePicture": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "building block Atlas_Web_Content.Alert": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.AlertIcon": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.AlertIcon_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Alert_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Breadcrumb": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Breadcrumb_Underline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_Action": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_ActionWithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Card_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Horizontal_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Form_Vertical_WithTitle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader_Background": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Heroheader_WithAction": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_DoubleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_SingleLine": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.ListItem_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.List_Cards": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.List_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Master_Detail": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.PageheaderImage_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithBack": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithControls": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Pageheader_WithSearch": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Timeline": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Timeline_WithImage": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Arrow": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Arrow_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Circle": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block Atlas_Web_Content.Wizard_Circle_Step": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "building block FeedbackModule.FeedbackWidget": {laws: []law{lawParse}, issue: "#721", why: "read-only, but describe prints a widget body that does not parse (#721 G)"}, + "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "a parameter comma is printed after its -- comment (#721 G)"}, + "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, + "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "the type parameter does not parse (#721 G)"}, // #721 E: menus — struck by ako/mxcli#980 (the menu item writer's missing // AlternativeText and Items). diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index cccdf57d3..0e0f2db99 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -122,30 +122,16 @@ var testAppKnownFailures = map[string]knownFailure{ "export mapping Services.OrdersExportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "export mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "import mapping Clients.SoapCustomerOrdersImportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "import mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "import mapping Clients.SoapOrdersImportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "import mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Reset_All_Filters": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Reset_Filter": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action DataWidgets.Set_Filter": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action FeedbackModule.JS_GetFeedbackStorageObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.DraftEmail": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.FindObjectWithGUID": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.Geocode": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.GetCurrentLocation": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.GetCurrentLocationMinimumAccuracy": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetGuid": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetObjectByGuid": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.GetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.RefreshEntity": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.RefreshObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ReverseGeocode": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.SetStorageItemObject": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "javascript action NanoflowCommons.SetStorageItemObjectList": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.SetStorageItemString": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.Share": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ShowConfirmation": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.ShowProgress": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action NanoflowCommons.SignIn": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "javascript action WebActions.TakePicture": {laws: []law{lawParse}, issue: "#721", why: "javascript action: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page Administration.Account_Overview": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page FeedbackModule.PopupFailure": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page FeedbackModule.PopupSuccess": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, From 6c24c64d022c1cf5ea5917fdba599eadf1eddeea Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 21:00:43 +0000 Subject: [PATCH 20/21] fix: refuse import from mapping without a result entity (mendixlabs/mxcli#1418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: "Exec writes an import-mapping microflow with a null entity and the `.mpr` can no longer be loaded" — a microflow calling `import from mapping X` above `create import mapping X` passed check and exec, then mx check failed loading the project with "Value cannot be null. (Parameter 'value')" in EntityType.set_EntityId. The flow builder dropped the mapping lookup error and stored the activity with VariableType.Entity = "". Fixed at three layers: - builder: an unresolved mapping, or one without a root entity, is a statement error instead of an empty result entity; - MDL-ORDER01 (no-project and project tiers) gains an "import mapping" kind, so check reports the forward reference before anything is written; - writer: CreateMicroflow / UpdateMicroflow and the splice's SerializeObject refuse an import activity with no result entity (loop bodies included). A project already written by the old bug was not repaired by re-running the statement: describe prints the broken activity as exactly the statement that would fix it, so create or modify reported "Unchanged microflow". A stored import activity without a result entity is now not spliceable: mdl 0 rebuilds the flow (healing it), mdl 1 refuses with the drop-and-create advice. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...mport-mapping-forward-ref-null-entity.json | 1 + .../1418-import-mapping-forward-ref.fail.mdl | 27 +++ .../modelsdk/microflow_import_entity_guard.go | 58 ++++++ .../microflow_import_entity_guard_test.go | 78 ++++++++ .../modelsdk/microflow_mutator_write.go | 3 + mdl/backend/modelsdk/microflow_write.go | 6 + mdl/executor/cmd_flow_modify.go | 37 ++++ mdl/executor/cmd_microflows_builder_calls.go | 16 +- mdl/executor/helpers.go | 18 ++ .../import_mapping_unresolved_pedapp_test.go | 171 ++++++++++++++++++ .../import_mapping_unresolved_test.go | 99 ++++++++++ mdl/executor/validate_definition_order.go | 17 +- 12 files changed, 528 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json create mode 100644 mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl create mode 100644 mdl/backend/modelsdk/microflow_import_entity_guard.go create mode 100644 mdl/backend/modelsdk/microflow_import_entity_guard_test.go create mode 100644 mdl/executor/import_mapping_unresolved_pedapp_test.go create mode 100644 mdl/executor/import_mapping_unresolved_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json new file mode 100644 index 000000000..719b48c7c --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-12-import-mapping-forward-ref-null-entity.json @@ -0,0 +1 @@ +{"date": "2026-10-12", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#1418: a script with `import from mapping X` in a microflow ABOVE `create import mapping X` passed check --references and exec ('Created microflow'), and mx check then could not load the project: InvalidOperationException setting 'Entity' of an Object type -> ArgumentNullException: Value cannot be null at Mendix.Modeler.DataTypes.EntityType.set_EntityId. Re-running the microflow once the mapping existed printed 'Unchanged microflow' and the project stayed unloadable.", "cause": "addImportFromMappingAction dropped the GetImportMappingByQualifiedName error and left ResultEntityID empty; importXmlActionToGen wrote VariableType.Entity unconditionally. MDL-ORDER01 had no import-mapping kind. On the re-run, describe prints the broken activity as exactly the statement that would repair it, so the create-or-modify statement diff found no change.", "file": "mdl/executor/cmd_microflows_builder_calls.go; mdl/executor/validate_definition_order.go; mdl/executor/helpers.go; mdl/executor/cmd_flow_modify.go; mdl/backend/modelsdk/microflow_import_entity_guard.go; mdl/backend/modelsdk/microflow_write.go; mdl/backend/modelsdk/microflow_mutator_write.go", "fix": "Builder refuses an unresolved mapping or one with no root entity; MDL-ORDER01 (both tiers) gains an 'import mapping' kind for import from mapping; Create/UpdateMicroflow and the splice SerializeObject refuse an ImportXmlAction with empty ResultEntityID (loop bodies walked); planFlowModify treats a stored import activity with no result entity as not spliceable, so mdl 0 rebuilds (heals) and mdl 1 refuses with drop-and-create advice instead of 'Unchanged'.", "test": "TestImportFromMappingRefusesUnresolvedMapping, TestValidateScriptDefinitionOrder_ImportMappingForwardRef, TestImportMappingForwardRefExecRefuses1418, TestImportMappingEmptyStoredEntityIsNotUnchanged1418, TestWriterRefusesImportMappingWithoutResultEntity; mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl", "insight": "**A dropped lookup error in a flow builder is a load error waiting to happen**: the value it would have produced is usually a typed identifier (here an EntityIdentifier), and empty is not 'unset' to Mendix, it is null into a setter. grep builders for `err == nil {` around backend lookups - the missing else is the bug. **Fixing the writer does not fix projects already written**: create-or-modify compares the declared statement with describe's rendering of the stored flow, and a value describe never prints (the result entity is inferred from the mapping, not stated) cannot differ, so the heal needs an explicit stored-shape check before the diff. Controls: revert builder+writer -> pedapp exec test stores Entity \"\"; revert the flow_modify check -> re-run prints 'Unchanged microflow: MyFirstModule.MF_D'. mx 11.6.6: old binary -> ArgumentNullException at load; fixed binary -> exec refused, mx check 0 errors; fixed mdl 0 re-run over the old-corrupted copy -> 'Replaced microflow', mx check 0 errors. Not covered: REST call `returns mapping X as E` with a forward mapping still writes (it states the entity itself; only ForceSingleOccurrence is inferred from the mapping).", "refs": ["mendixlabs/mxcli#1418"]} diff --git a/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl b/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl new file mode 100644 index 000000000..ea1d4707d --- /dev/null +++ b/mdl-examples/bug-tests/1418-import-mapping-forward-ref.fail.mdl @@ -0,0 +1,27 @@ +-- Bug mendixlabs/mxcli#1418: exec writes an import-mapping microflow with a +-- null entity and the .mpr can no longer be loaded. +-- +-- NEGATIVE TEST (.fail.mdl) — EXPECTED to fail `mxcli check` (MDL-ORDER01). +-- +-- Symptom: `check --references` and `exec` both succeeded; the microflow was +-- stored with VariableType.Entity = "" because the mapping did not exist yet +-- when the flow was built, and `mx check` then died loading the project: +-- ArgumentNullException: Value cannot be null. (Parameter 'value') +-- at Mendix.Modeler.DataTypes.EntityType.set_EntityId +-- +-- Fix: MDL-ORDER01 reports the forward reference; exec refuses an +-- `import from mapping` whose mapping does not resolve; the writer refuses an +-- import activity without a result entity. +-- +-- Correct form: move `create import mapping` above the microflow. + +create json structure MyFirstModule.JSON_D sample '{"code": "NL", "name": "Netherlands"}'; +create persistent entity MyFirstModule.CountryD ( Code: string, Name: string ); + +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; + +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code, Name = name } }; diff --git a/mdl/backend/modelsdk/microflow_import_entity_guard.go b/mdl/backend/modelsdk/microflow_import_entity_guard.go new file mode 100644 index 000000000..f28fb3853 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_import_entity_guard.go @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// importMappingEntityError refuses an import-from-mapping activity without a +// result entity. importXmlActionToGen stores the entity as the activity's +// VariableType.Entity, and an empty one cannot be loaded: Mendix rebuilds it +// into an EntityIdentifier as it reads the unit and `mx check` dies with +// ArgumentNullException in EntityType.set_EntityId before validating anything +// (mendixlabs/mxcli#1418). The builder refuses a mapping it cannot resolve; +// this is the last line, for every path that reaches the encoder. Loop bodies +// are walked, as their activities are written the same way. +func importMappingEntityError(obj microflows.MicroflowObject) error { + switch o := obj.(type) { + case *microflows.ActionActivity: + ix, ok := o.Action.(*microflows.ImportXmlAction) + if !ok { + return nil + } + if ix.ResultHandling == nil || ix.ResultHandling.ResultEntityID == "" { + mapping := "(none)" + if ix.ResultHandling != nil && ix.ResultHandling.MappingID != "" { + mapping = string(ix.ResultHandling.MappingID) + } + return fmt.Errorf("refusing to write import from mapping %s: the activity has no result entity, "+ + "and Mendix cannot load a project holding one (EntityType.set_EntityId: value cannot be null). "+ + "The import mapping must exist, with a root entity, before the flow that calls it is written", mapping) + } + case *microflows.LoopedActivity: + if o.ObjectCollection != nil { + for _, inner := range o.ObjectCollection.Objects { + if err := importMappingEntityError(inner); err != nil { + return err + } + } + } + } + return nil +} + +// flowImportMappingEntityError applies importMappingEntityError to a flow's objects. +func flowImportMappingEntityError(oc *microflows.MicroflowObjectCollection) error { + if oc == nil { + return nil + } + for _, obj := range oc.Objects { + if err := importMappingEntityError(obj); err != nil { + return err + } + } + return nil +} diff --git a/mdl/backend/modelsdk/microflow_import_entity_guard_test.go b/mdl/backend/modelsdk/microflow_import_entity_guard_test.go new file mode 100644 index 000000000..5a67acc25 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_import_entity_guard_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// mendixlabs/mxcli#1418: an import-from-mapping activity stored with an empty +// VariableType.Entity makes the whole project unloadable: +// +// System.InvalidOperationException: An error occurred when trying to set the +// 'Entity' property of a Object type in a Microflow with ID ... +// ---> System.ArgumentNullException: Value cannot be null. (Parameter 'value') +// at Mendix.Modeler.DataTypes.EntityType.set_EntityId(EntityIdentifier value) +// +// (mx check, 11.6.6). Whatever the caller hands it, the writer refuses rather +// than store one — including inside a loop body, and on the splice path. +func importActivity(entity model.ID) *microflows.ActionActivity { + return µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "act-1"}, + }}, + Action: µflows.ImportXmlAction{ + BaseElement: model.BaseElement{ID: "imp-1"}, + XmlDocumentVariable: "P", + ResultHandling: µflows.ResultHandlingMapping{ + BaseElement: model.BaseElement{ID: "rh-1"}, + MappingID: "MyFirstModule.IMM_D", + ResultEntityID: entity, + ResultVariable: "R", + SingleObject: true, + }, + }, + } +} + +func TestWriterRefusesImportMappingWithoutResultEntity(t *testing.T) { + inLoop := µflows.LoopedActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{BaseElement: model.BaseElement{ID: "loop-1"}}, + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{importActivity("")}, + }, + } + for name, obj := range map[string]microflows.MicroflowObject{ + "top level": importActivity(""), + "in a loop": inLoop, + } { + t.Run(name, func(t *testing.T) { + mf := µflows.Microflow{ + BaseElement: model.BaseElement{ID: "mf-1"}, + Name: "MF_D", + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{obj}}, + } + b := &Backend{} + for op, err := range map[string]error{ + "CreateMicroflow": b.CreateMicroflow(mf), + "UpdateMicroflow": b.UpdateMicroflow(mf), + } { + if err == nil || !strings.Contains(err.Error(), "MyFirstModule.IMM_D") || !strings.Contains(err.Error(), "result entity") { + t.Errorf("%s: err = %v, want a refusal naming the mapping's missing result entity", op, err) + } + } + if _, err := (codecMicroflowDeps{b: b}).SerializeObject(obj); err == nil { + t.Errorf("SerializeObject: no error for an import activity without a result entity") + } + }) + } + + // Control: with the entity the guard stays out of the way. + if err := importMappingEntityError(importActivity("MyFirstModule.CountryD")); err != nil { + t.Fatalf("activity with a result entity refused: %v", err) + } +} diff --git a/mdl/backend/modelsdk/microflow_mutator_write.go b/mdl/backend/modelsdk/microflow_mutator_write.go index 43ff761ee..d3836cf40 100644 --- a/mdl/backend/modelsdk/microflow_mutator_write.go +++ b/mdl/backend/modelsdk/microflow_mutator_write.go @@ -69,6 +69,9 @@ func (d codecMicroflowDeps) DeclaresProperty(docType, key string) bool { } func (d codecMicroflowDeps) SerializeObject(obj microflows.MicroflowObject) (bson.D, error) { + if err := importMappingEntityError(obj); err != nil { + return nil, err + } el := microflowObjectToGen(obj) if el == nil { return nil, nil diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 11e12fc60..b70053c2d 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -148,6 +148,9 @@ func (b *Backend) CreateMicroflow(mf *microflows.Microflow) error { if mf == nil { return fmt.Errorf("CreateMicroflow: nil microflow") } + if err := flowImportMappingEntityError(mf.ObjectCollection); err != nil { + return fmt.Errorf("CreateMicroflow %s: %w", mf.Name, err) + } if b.writer == nil { return fmt.Errorf("CreateMicroflow: not connected for writing") } @@ -172,6 +175,9 @@ func (b *Backend) UpdateMicroflow(mf *microflows.Microflow) error { if mf == nil { return fmt.Errorf("UpdateMicroflow: nil microflow") } + if err := flowImportMappingEntityError(mf.ObjectCollection); err != nil { + return fmt.Errorf("UpdateMicroflow %s: %w", mf.Name, err) + } if b.writer == nil { return fmt.Errorf("UpdateMicroflow: not connected for writing") } diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index 756b66ddc..0d96cb32d 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -237,6 +237,17 @@ func planFlowModify(ctx *ExecContext, d *flowDecl) (*flowPlan, error) { } return nil, cannotSplice("the stored %s cannot be read for splicing: %v", d.kind(), err) } + // A stored import activity without its result entity (what mxcli wrote + // before mendixlabs/mxcli#1418 for a mapping created further down the + // script) makes the project unloadable, and describe prints it exactly as + // the statement that would repair it, so the statement diff finds nothing + // and reported "Unchanged" over a broken project. Treat it as a change the + // splice cannot make: mdl 0 rebuilds the flow — resolving the mapping + // afresh — and mdl 1 refuses with the drop-and-create advice, rather than + // keeping it silently. + if mapping := storedImportWithoutEntity(a.mf.ObjectCollection); mapping != "" { + return nil, cannotSplice("its stored `import from mapping %s` activity has no result entity, which Mendix cannot load (mendixlabs/mxcli#1418)", mapping) + } stored, err := describedFlowStmt(ctx, d, a) if err != nil { @@ -378,6 +389,32 @@ func fallBack(ctx *ExecContext, d *flowDecl, v flowVerdict) (bool, error) { return false, nil } +// storedImportWithoutEntity returns the mapping of the first import-from-mapping +// activity in oc (loop bodies included) whose result entity is empty, or "". +func storedImportWithoutEntity(oc *microflows.MicroflowObjectCollection) string { + if oc == nil { + return "" + } + for _, obj := range oc.Objects { + switch o := obj.(type) { + case *microflows.ActionActivity: + if ix, ok := o.Action.(*microflows.ImportXmlAction); ok { + if ix.ResultHandling == nil { + return "(none)" + } + if ix.ResultHandling.ResultEntityID == "" { + return string(ix.ResultHandling.MappingID) + } + } + case *microflows.LoopedActivity: + if m := storedImportWithoutEntity(o.ObjectCollection); m != "" { + return m + } + } + } + return "" +} + // reportUnchanged reports a statement that wrote nothing, collapsing into the // run's summary like an elided write does. func reportUnchanged(ctx *ExecContext, what string) { diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index dff6a7f7d..2eb033263 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -2096,15 +2096,29 @@ func (fb *flowBuilder) addImportFromMappingAction(s *ast.ImportFromMappingStmt) // JSON structure check covers JSON-backed mappings; for XML schema or // message-definition mappings JsonStructure is empty and the root // element kind on the mapping itself indicates Array vs Object. + // + // A mapping that does not resolve is refused, not written around. The + // result entity is stored as the activity's VariableType.Entity, and an + // empty one makes the whole project unloadable — `mx check` dies with + // ArgumentNullException in EntityType.set_EntityId before validating + // anything (mendixlabs/mxcli#1418). The usual cause is a script that calls + // the mapping above its `create import mapping`; MDL-ORDER01 reports that + // at check time, this is the exec-time half. resultEntityQN := "" if fb.backend != nil { - if im, err := fb.backend.GetImportMappingByQualifiedName(s.Mapping.Module, s.Mapping.Name); err == nil { + im, err := fb.backend.GetImportMappingByQualifiedName(s.Mapping.Module, s.Mapping.Name) + switch { + case err != nil || im == nil: + fb.addError("import from mapping %s: import mapping not found in the project (create it above this microflow; an activity without its result entity makes the project unloadable)", s.Mapping.String()) + default: if !rangeAuthored && fb.mappingRootIsList(im) { resultHandling.SingleObject = false } if len(im.Elements) > 0 && im.Elements[0] != nil && im.Elements[0].Entity != "" { resultEntityQN = im.Elements[0].Entity resultHandling.ResultEntityID = model.ID(resultEntityQN) + } else { + fb.addError("import from mapping %s: the import mapping has no root entity, so the activity's result type cannot be written (an empty one makes the project unloadable)", s.Mapping.String()) } } } diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index 4e984d8c6..b69e4f000 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -598,6 +598,24 @@ func buildQueueQualifiedNames(ctx *ExecContext) map[string]bool { return result } +// buildImportMappingQualifiedNames returns the set of import mapping qualified +// names in the project, matched exactly as GetImportMappingByQualifiedName does. +func buildImportMappingQualifiedNames(ctx *ExecContext) map[string]bool { + result := make(map[string]bool) + h, err := getHierarchy(ctx) + if err != nil { + return result + } + ims, err := ctx.Backend.ListImportMappings() + if err != nil { + return result + } + for _, im := range ims { + result[h.GetQualifiedName(im.ContainerID, im.Name)] = true + } + return result +} + // buildNanoflowQualifiedNames returns a set of all nanoflow qualified names in the project. func buildNanoflowQualifiedNames(ctx *ExecContext) map[string]bool { result := make(map[string]bool) diff --git a/mdl/executor/import_mapping_unresolved_pedapp_test.go b/mdl/executor/import_mapping_unresolved_pedapp_test.go new file mode 100644 index 000000000..84b4ee02e --- /dev/null +++ b/mdl/executor/import_mapping_unresolved_pedapp_test.go @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/model" +) + +const imm1418Prelude = `create json structure MyFirstModule.JSON_D sample '{"code": "NL", "name": "Netherlands"}'; +create persistent entity MyFirstModule.CountryD ( Code: string, Name: string ); +` + +const imm1418Microflow = `create or modify microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; +` + +const imm1418Mapping = `create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code, Name = name } }; +` + +// mfUnitByName returns a microflow's unit ID and stored bytes. +func mfUnitByName(t *testing.T, exec *Executor, module, name string) (model.ID, []byte) { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListMicroflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name && h.GetModuleName(h.FindModuleID(m.ContainerID)) == module { + raw, err := ctx.Backend.GetRawUnitBytes(m.ID) + if err != nil { + t.Fatal(err) + } + return m.ID, append([]byte(nil), raw...) + } + } + return "", nil +} + +// entityTypeEntities collects the Entity of every DataTypes$ObjectType / +// ListType in a stored document, and when set != nil replaces it. +func entityTypeEntities(v any, set *string, out *[]string) any { + switch x := v.(type) { + case bson.D: + typ, _ := afGet(x, "$Type").(string) + isEntityType := typ == "DataTypes$ObjectType" || typ == "DataTypes$ListType" + for i := range x { + if isEntityType && x[i].Key == "Entity" { + s, _ := x[i].Value.(string) + *out = append(*out, s) + if set != nil { + x[i].Value = *set + } + continue + } + x[i].Value = entityTypeEntities(x[i].Value, set, out) + } + return x + case bson.A: + for i := range x { + x[i] = entityTypeEntities(x[i], set, out) + } + return x + } + return v +} + +// mendixlabs/mxcli#1418, at the layer the symptom lives in: a real project +// write. Exec of a microflow calling a mapping created further down the script +// used to "Create microflow" with VariableType.Entity = "", which mx check +// cannot load (ArgumentNullException in EntityType.set_EntityId). +func TestImportMappingForwardRefExecRefuses1418(t *testing.T) { + exec, _ := openPedAppFixture(t) + err := agreeExec(t, exec, "mdl 1;\n"+imm1418Prelude+imm1418Microflow+imm1418Mapping) + if err == nil || !strings.Contains(err.Error(), "import mapping not found") { + var got []string + if _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D"); raw != nil { + var d bson.D + if uerr := bson.Unmarshal(raw, &d); uerr == nil { + entityTypeEntities(d, nil, &got) + } + } + t.Fatalf("exec err = %v, want the import mapping refused; stored result entities: %q", err, got) + } + if id, _ := mfUnitByName(t, exec, "MyFirstModule", "MF_D"); id != "" { + t.Fatalf("the refused microflow was written anyway (%s)", id) + } + + // Control: the same statements in definition order execute. + if err := agreeExec(t, exec, "mdl 1;\n"+imm1418Mapping+imm1418Microflow); err != nil { + t.Fatalf("control: mapping-first order failed: %v", err) + } + _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + var got []string + entityTypeEntities(d, nil, &got) + if len(got) != 1 || got[0] != "MyFirstModule.CountryD" { + t.Fatalf("stored result entities = %q, want [MyFirstModule.CountryD]", got) + } +} + +// A project already written by the old bug: re-running the statement once the +// mapping exists printed "Unchanged microflow" and kept the empty entity, +// because describe prints the broken activity as the very statement that would +// repair it. mdl 0 now rebuilds it (healing the project); mdl 1 refuses with +// the reason instead of reporting Unchanged. +func TestImportMappingEmptyStoredEntityIsNotUnchanged1418(t *testing.T) { + exec, out := openPedAppFixture(t) + if err := agreeExec(t, exec, "mdl 1;\n"+imm1418Prelude+imm1418Mapping+imm1418Microflow); err != nil { + t.Fatal(err) + } + corrupt := func() { + t.Helper() + id, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + empty := "" + var got []string + entityTypeEntities(d, &empty, &got) + if len(got) != 1 { + t.Fatalf("expected one entity type in MF_D, found %q", got) + } + b, err := bson.Marshal(d) + if err != nil { + t.Fatal(err) + } + if err := exec.newExecContext(context.Background()).Backend.UpdateRawUnit(string(id), b); err != nil { + t.Fatal(err) + } + } + + corrupt() + out.Reset() + err := agreeExec(t, exec, "mdl 1;\n"+imm1418Microflow) + if err == nil || !strings.Contains(err.Error(), "no result entity") { + t.Fatalf("mdl 1 re-run over a stored empty entity: err = %v, output %q; want a refusal naming the missing result entity", err, out.String()) + } + + out.Reset() + if err := agreeExec(t, exec, imm1418Microflow); err != nil { + t.Fatalf("mdl 0 re-run: %v", err) + } + _, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_D") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + var got []string + entityTypeEntities(d, nil, &got) + if len(got) != 1 || got[0] != "MyFirstModule.CountryD" { + t.Fatalf("after the mdl 0 re-run the stored result entities are %q, want [MyFirstModule.CountryD] (output %q)", got, out.String()) + } +} diff --git a/mdl/executor/import_mapping_unresolved_test.go b/mdl/executor/import_mapping_unresolved_test.go new file mode 100644 index 000000000..519327b7b --- /dev/null +++ b/mdl/executor/import_mapping_unresolved_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "errors" + "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" +) + +// mendixlabs/mxcli#1418: `import from mapping X` written before `create import +// mapping X` stored the activity with VariableType.Entity = "", and Mendix +// cannot load the project ("ArgumentNullException ... EntityType.set_EntityId"). +// The lookup error was dropped and an empty result entity written. The builder +// must refuse the statement instead. +func TestImportFromMappingRefusesUnresolvedMapping(t *testing.T) { + tests := []struct { + name string + lookup func(moduleName, name string) (*model.ImportMapping, error) + want string + }{ + { + name: "mapping not found", + lookup: func(moduleName, name string) (*model.ImportMapping, error) { + return nil, errors.New("import mapping not found: " + moduleName + "." + name) + }, + want: "not found", + }, + { + name: "mapping has no root entity", + lookup: func(moduleName, name string) (*model.ImportMapping, error) { + return &model.ImportMapping{Elements: []*model.ImportMappingElement{{Kind: "Object"}}}, nil + }, + want: "no root entity", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fb := &flowBuilder{ + varTypes: map[string]string{}, + backend: &mock.MockBackend{GetImportMappingByQualifiedNameFunc: tt.lookup}, + } + fb.addImportFromMappingAction(&ast.ImportFromMappingStmt{ + OutputVariable: "R", + SourceVariable: "P", + Mapping: ast.QualifiedName{Module: "MyFirstModule", Name: "IMM_D"}, + }) + if len(fb.errors) == 0 { + var entity model.ID + for _, o := range fb.objects { + if act, ok := o.(*microflows.ActionActivity); ok { + if ix, ok := act.Action.(*microflows.ImportXmlAction); ok && ix.ResultHandling != nil { + entity = ix.ResultHandling.ResultEntityID + } + } + } + t.Fatalf("no builder error; activity written with ResultEntityID=%q (Mendix cannot load an empty entity)", entity) + } + msg := strings.Join(fb.errors, "\n") + if !strings.Contains(msg, "MyFirstModule.IMM_D") || !strings.Contains(msg, tt.want) { + t.Fatalf("error = %q, want it to name MyFirstModule.IMM_D and %q", msg, tt.want) + } + }) + } +} + +// MDL-ORDER01 must report the forward reference at check time, before exec +// writes the statements ahead of it. +func TestValidateScriptDefinitionOrder_ImportMappingForwardRef(t *testing.T) { + msgs := buildOrderProgram(t, `create json structure MyFirstModule.JSON_D sample '{"code": "NL"}'; +create persistent entity MyFirstModule.CountryD ( Code: string ); +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end; +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code } };`) + if len(msgs) != 1 || !strings.Contains(msgs[0], "import mapping MyFirstModule.IMM_D") { + t.Fatalf("violations = %q, want one naming import mapping MyFirstModule.IMM_D", msgs) + } + + // The backward order is fine. + msgs = buildOrderProgram(t, `create json structure MyFirstModule.JSON_D sample '{"code": "NL"}'; +create persistent entity MyFirstModule.CountryD ( Code: string ); +create import mapping MyFirstModule.IMM_D with json structure MyFirstModule.JSON_D +{ create MyFirstModule.CountryD { Code = code } }; +create microflow MyFirstModule.MF_D ( $P: string ) +begin + $R = import from mapping MyFirstModule.IMM_D($P); +end;`) + if len(msgs) != 0 { + t.Fatalf("backward reference flagged: %q", msgs) + } +} diff --git a/mdl/executor/validate_definition_order.go b/mdl/executor/validate_definition_order.go index 395afa6e9..0fec65d31 100644 --- a/mdl/executor/validate_definition_order.go +++ b/mdl/executor/validate_definition_order.go @@ -38,6 +38,10 @@ const ( defNanoflow = "nanoflow" defPage = "page" defQueue = "task queue" + // defImportMapping is the target of `import from mapping`, which the flow + // builder resolves when it writes the activity — it needs the mapping's + // root entity for the result type (mendixlabs/mxcli#1418). + defImportMapping = "import mapping" ) // defKey identifies a document by kind and qualified name. Kind is part of the @@ -160,6 +164,8 @@ func definedBy(stmt ast.Statement) (key defKey, plain bool) { return qualifiedKey(defPage, s.Name), !s.IsModify && !s.IsReplace case *ast.CreateQueueStmt: return qualifiedKey(defQueue, s.Name), !s.CreateOrModify + case *ast.CreateImportMappingStmt: + return qualifiedKey(defImportMapping, s.Name), !s.CreateOrModify } return defKey{}, false } @@ -271,6 +277,10 @@ func flowDefRefs(site string, params []ast.MicroflowParam, ret *ast.MicroflowRet // The action itself resolves lazily (measured, #955); its IN QUEUE // does not — buildQueueSettings refuses a queue it cannot find. out = append(out, queueDefRef(c.Queue, site)...) + case *ast.ImportFromMappingStmt: + // Resolved for its root entity, which the activity stores as its + // result type; exec refuses one it cannot find (#1418). + out = append(out, oneDefRef(c.Mapping, defImportMapping, site+": import from mapping")...) } } return out @@ -424,6 +434,8 @@ func validateForwardDefRefs(ctx *ExecContext, prog *ast.Program) []error { m = buildPageQualifiedNames(ctx) case defQueue: m = buildQueueQualifiedNames(ctx) + case defImportMapping: + m = buildImportMappingQualifiedNames(ctx) } stored[kind] = m return m @@ -432,8 +444,9 @@ func validateForwardDefRefs(ctx *ExecContext, prog *ast.Program) []error { m := storedOf(kind) // An empty listing is a backend that could not answer, as often as a // project without any — silence, not a finding (cf. checkAlterTarget). - // Queues are the exception: a project with none is ordinary. - if len(m) == 0 && kind != defQueue { + // Queues and import mappings are the exception: a project with none + // is ordinary. + if len(m) == 0 && kind != defQueue && kind != defImportMapping { return true } if kind == defQueue { From 678c8d0762d7ef0fa2b5f436b578c1a40af04344 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 21:11:38 +0000 Subject: [PATCH 21/21] fix: write flows in the shape the project's Mendix version stores (mendixlabs/mxcli#1410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: microflows and nanoflows cannot be created or altered in a 9.x project — Error: failed to create microflow: unit …: this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+) (also CreateNanoflow and ALTER MICROFLOW; 10.0–10.14 for CaseValues alone). Same root cause as mendixlabs/mxcli#1434, whose repro is an empty microflow on 9.24.2. The flow writers emitted the newest shape whatever the version. The property floor (e671bec3) rightly refuses the two keys that hold a value; before it they were written silently, and forcing that shape back onto 9.24 is 8 errors in mx check (CE0079/CE0773: the decision's cases are not read). Gated on major.minor (flowVersion), with the Model SDK's StructureVersionInfo (mendixmodelsdk 4.116.0) as the floors and the shapes measured from a Studio Pro 9.24.30 document and the same document after `mx convert` to 10.12.19: - Microflow.StableId only from 10.2 (encoder OmitKeys, which now also applies to registered type defaults); on >= 10.2 it is emitted and carried as before - the case as one NewCaseValue below 10.15, a CaseValues list from 10.15 (gen bound the single case as "CaseValue"; overridden to its storage name) - Origin/DestinationBezierVector below 10.16 (+ LineType "BezierCurve" from 10.8), the BezierCurve Line from 10.16 — sequence and annotation flows; the reader takes the vectors from either shape - ReturnVariableName from 10.12, parameter IsRequired/DefaultValue from 10.17 (both were written on any 10.x; the floor cannot see them) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...10-12-flow-writes-refused-on-mendix-9.json | 1 + .../bug-tests/1410-flows-on-mendix-9.mdl | 53 ++++ .../modelsdk/flow_version_keys_test.go | 236 ++++++++++++++++++ .../modelsdk/flow_version_shape_test.go | 129 ++++++++++ mdl/backend/modelsdk/microflow.go | 5 + .../modelsdk/microflow_annotation_test.go | 2 +- .../modelsdk/microflow_idempotence_test.go | 2 +- .../modelsdk/microflow_mutator_write.go | 16 +- .../microflow_parameter_position_test.go | 6 +- mdl/backend/modelsdk/microflow_readback.go | 7 +- .../microflow_roundtrip_flags_test.go | 4 +- .../microflow_rule_split_write_test.go | 2 +- mdl/backend/modelsdk/microflow_write.go | 152 ++++++++--- mdl/backend/modelsdk/nanoflow_carry_test.go | 4 +- mdl/backend/modelsdk/nanoflow_write.go | 23 +- mdl/backend/modelsdk/rule_write.go | 21 +- mdl/backend/modelsdk/rule_write_test.go | 4 +- modelsdk/codec/decoder.go | 3 + modelsdk/codec/encoder.go | 14 +- modelsdk/gen/microflows/types.go | 10 +- 20 files changed, 608 insertions(+), 86 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json create mode 100644 mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl create mode 100644 mdl/backend/modelsdk/flow_version_keys_test.go create mode 100644 mdl/backend/modelsdk/flow_version_shape_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json new file mode 100644 index 000000000..bf96fb6dc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-12-flow-writes-refused-on-mendix-9.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-12", "refs": ["mendixlabs/mxcli#1410", "mendixlabs/mxcli#1434"], "symptom": "Every create microflow / create nanoflow / alter microflow on a Mendix 9.x project failed after earlier statements had already been written: \"this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+)\" (10.0-10.14: CaseValues alone), while `mxcli check` passed the script", "cause": "The flow writers emitted the newest shape whatever the version, gated by major at most: StableId as an unconditional FreshGUIDFields codec default (10.2+), the case always as a CaseValues list (10.15+), always a BezierCurve Line (10.16+), ReturnVariableName and parameter IsRequired/DefaultValue on any 10.x (10.12+ / 10.17+). The property floor added on 2026-10-10 started refusing the two that held a value; before it they were written silently, and on 9.24 the case list is not read at all (forced back in: 8 errors, CE0079/CE0773). Line, LineType, ReturnVariableName and the parameter keys were never refused because the floor looks a key up on the element's own $Type only and these are declared on supertypes (Flow, MicroflowBase) or under another $Type (MicroflowParameterObject). And gen bound the single case under its SDK name \"CaseValue\"; Studio Pro stores it as \"NewCaseValue\"", "file": "`mdl/backend/modelsdk/microflow_write.go` (flowVersion, microflowEncoder, sequenceFlowToGen, annotationFlowToGen, microflowParameterToGen), `nanoflow_write.go`, `rule_write.go`, `microflow_mutator_write.go`, `microflow_readback.go`, `microflow.go` (flowsFromGen reads the bezier vectors), `modelsdk/gen/microflows/types.go` (initSequenceFlow storage-name override), `modelsdk/codec/encoder.go` (OmitKeys applies to registered defaults)", "insight": "Get the old shape from a real document, not the gen: dump a Studio Pro-authored 9.x flow (any marketplace module's microflow with a decision) and run `mx convert -p` to a mid-10 version (10.12.19 here) to get a Studio Pro-serialized document for that version for free — its key sets settle LineType (10.8-10.15 only), NewCaseValue, StableId and the absent parameter keys in one dump, and mxcli's output can be diffed key-set for key-set against it. The floor (canon.StripUndeclaredProperties) is a backstop, not a gate: it cannot see supertype-declared keys, so a writer clean under the floor can still write keys the version lacks — check the Model SDK's versionInfo for the type AND its supertypes. A refusal at exec time leaves earlier statements written; #1434 (check is not version-aware) is the same root cause and is cured by the writer emitting a storable shape, not by teaching check the floor."} diff --git a/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl b/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl new file mode 100644 index 000000000..6518ef44a --- /dev/null +++ b/mdl-examples/bug-tests/1410-flows-on-mendix-9.mdl @@ -0,0 +1,53 @@ +mdl 1; +-- mendixlabs/mxcli#1410 — microflows and nanoflows could not be created or +-- altered in a Mendix 9 project (and 10.0–10.14 for any flow): +-- +-- this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId +-- (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+) +-- +-- The flow writers now emit the shape the project's version stores: no +-- StableId below 10.2, the decision case as a single NewCaseValue below 10.15, +-- Origin/DestinationBezierVector instead of a BezierCurve Line below 10.16. +-- Measured: this script on a Studio Pro 9.24.30 app and on the same app after +-- `mx convert` to 10.12.19 checks at 0 errors; the old shape forced onto 9.24 +-- is 8 errors (CE0079 / CE0773 — the decision's cases are not read). +-- +-- Verify (on a copy of a Mendix 9.x or 10.0–10.14 project): +-- mxcli exec 1410-flows-on-mendix-9.mdl -p app.mpr +-- mxcli docker check -p app.mpr + +create module R1410; + +create entity R1410.Item ( + "Count": integer +); + +create microflow R1410.MF_Empty () +begin + return; +end; + +create microflow R1410.MF_Decide ($Item: R1410.Item) returns boolean +begin + declare $Result boolean = false; + if $Item/"Count" > 10 then + set $Result = true; + log info node 'R1410' 'big'; + else + log info node 'R1410' 'small'; + end if; + return $Result; +end; + +create nanoflow R1410.NF_Decide ($Item: R1410.Item) returns boolean +begin + if $Item/"Count" > 10 then + return true; + else + return false; + end if; +end; + +alter microflow R1410.MF_Decide { + insert after $Result begin log info node 'R1410' 'declared'; end; +}; diff --git a/mdl/backend/modelsdk/flow_version_keys_test.go b/mdl/backend/modelsdk/flow_version_keys_test.go new file mode 100644 index 000000000..1203b2d2d --- /dev/null +++ b/mdl/backend/modelsdk/flow_version_keys_test.go @@ -0,0 +1,236 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "path/filepath" + "testing" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// mendixlabs/mxcli#1410: on a Mendix 9 project every flow write was refused — +// +// this Mendix 9.24.2 project cannot store Microflows$Microflow.StableId +// (Mendix 10.2.0+); Microflows$SequenceFlow.CaseValues (Mendix 10.15.0+) +// +// The flow writers emitted the newest shape whatever the project's version: +// a StableId (introduced 10.2.0), the CaseValues list (introduced 10.15.0, +// replacing the single case stored as NewCaseValue) and a BezierCurve Line +// (introduced 10.16.0, replacing Origin/DestinationBezierVector). The property +// floor (canon.StripUndeclaredProperties) is right to refuse a key the version +// does not declare when it holds a value; the writer has to emit the shape the +// version has. The pre-10.15 shape is measured from a Studio Pro 9.24.30 +// document (Administration.ChangeMyPassword): +// +// {"$Type": "Microflows$SequenceFlow", "DestinationBezierVector": "-30;0", +// "DestinationConnectionIndex": 3, "DestinationPointer": …, "IsErrorHandler": false, +// "NewCaseValue": {"$Type": "Microflows$EnumerationCase", "Value": "true"}, +// "OriginBezierVector": "15;0", "OriginConnectionIndex": 1, "OriginPointer": …} +// +// and the version floors are the Model SDK's (mendixmodelsdk 4.116.0, +// StructureVersionInfo): Microflow.stableId 10.2.0, SequenceFlow.caseValue +// deleted / caseValues introduced 10.15.0, Flow.line 10.16.0. + +// copyV9Fixture copies the Mendix 9.24 v1 project the modelsdk/mpr tests use. +func copyV9Fixture(t *testing.T) string { + t.Helper() + dst := t.TempDir() + if err := os.CopyFS(dst, os.DirFS("../../../modelsdk/mpr/testdata/v1-project")); err != nil { + t.Fatalf("copy fixture: %v", err) + } + return filepath.Join(dst, "App.mpr") +} + +// decisionFlow builds start -> decision -(true)-> end, -(false)-> end, with a +// hand-drawn curve on the true branch. +func decisionFlow() *microflows.MicroflowObjectCollection { + obj := func(id string, x int) microflows.BaseMicroflowObject { + return microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(id)}, + Position: model.Point{X: x, Y: 100}, + Size: model.Size{Width: 30, Height: 30}, + } + } + start := µflows.StartEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000001", 0)} + split := µflows.ExclusiveSplit{ + BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000002", 100), + Caption: "big?", + SplitCondition: µflows.ExpressionSplitCondition{Expression: "1 > 0"}, + } + endT := µflows.EndEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000003", 200)} + endF := µflows.EndEvent{BaseMicroflowObject: obj("a1410000-0000-0000-0000-000000000004", 300)} + flow := func(id, from, to string, cv microflows.CaseValue) *microflows.SequenceFlow { + return µflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID(id)}, + OriginID: model.ID(from), DestinationID: model.ID(to), + OriginConnectionIndex: 1, DestinationConnectionIndex: 3, + CaseValue: cv, + } + } + curved := flow("a1410000-0000-0000-0000-000000000012", string(split.ID), string(endT.ID), µflows.EnumerationCase{Value: "true"}) + curved.OriginControlVector, curved.DestinationControlVector = "15;0", "-30;0" + return µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{start, split, endT, endF}, + Flows: []*microflows.SequenceFlow{ + flow("a1410000-0000-0000-0000-000000000011", string(start.ID), string(split.ID), nil), + curved, + flow("a1410000-0000-0000-0000-000000000013", string(split.ID), string(endF.ID), µflows.EnumerationCase{Value: "false"}), + }, + } +} + +// assertPre1015Flows checks every sequence flow of a stored flow document has +// the Mendix 9 shape: a single NewCaseValue, bezier vectors, no Line. +func assertPre1015Flows(t *testing.T, raw bson.Raw, wantCases map[string]string) { + t.Helper() + flows, ok := raw.Lookup("Flows").ArrayOK() + if !ok { + t.Fatalf("no Flows list") + } + vals, _ := flows.Values() + n := 0 + for _, v := range vals { + d, ok := v.DocumentOK() + if !ok { + continue + } + if typ, _ := d.Lookup("$Type").StringValueOK(); typ != "Microflows$SequenceFlow" { + continue + } + n++ + for _, key := range []string{"CaseValues", "Line", "LineType"} { + if _, err := d.LookupErr(key); err == nil { + t.Errorf("sequence flow carries %s, which Mendix 9 does not declare", key) + } + } + cv, ok := d.Lookup("NewCaseValue").DocumentOK() + if !ok { + t.Errorf("sequence flow has no NewCaseValue document — the case Mendix 9 stores") + continue + } + for _, key := range []string{"OriginBezierVector", "DestinationBezierVector"} { + if _, ok := d.Lookup(key).StringValueOK(); !ok { + t.Errorf("sequence flow has no %s", key) + } + } + id := blobID(d.Lookup("$ID")) + typ, _ := cv.Lookup("$Type").StringValueOK() + val, _ := cv.Lookup("Value").StringValueOK() + if want, ok := wantCases[id]; ok { + if typ != "Microflows$EnumerationCase" || val != want { + t.Errorf("flow %s case = %s %q, want EnumerationCase %q", id, typ, val, want) + } + if want == "true" { + ov, _ := d.Lookup("OriginBezierVector").StringValueOK() + dv, _ := d.Lookup("DestinationBezierVector").StringValueOK() + if ov != "15;0" || dv != "-30;0" { + t.Errorf("curved flow vectors = %q/%q, want 15;0/-30;0", ov, dv) + } + } + } else if typ != "Microflows$NoCase" { + t.Errorf("flow %s case = %s, want NoCase", id, typ) + } + } + if n != 3 { + t.Errorf("stored %d sequence flows, want 3", n) + } +} + +func blobID(v bson.RawValue) string { + _, data, ok := v.BinaryOK() + if !ok { + return "" + } + return types.BlobToUUID(data) +} + +func TestFlowWritesOnMendix9_1410(t *testing.T) { + b := New() + if err := b.Connect(copyV9Fixture(t)); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + if pv := b.ProjectVersion(); pv == nil || pv.MajorVersion != 9 { + t.Fatalf("fixture version = %+v, want a Mendix 9 project", pv) + } + mod, err := b.GetModuleByName("NanoflowCommons") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + cases := map[string]string{ + "a1410000-0000-0000-0000-000000000012": "true", + "a1410000-0000-0000-0000-000000000013": "false", + } + + t.Run("create microflow", func(t *testing.T) { + mf := µflows.Microflow{Name: "MF_Decide1410", ObjectCollection: decisionFlow()} + mf.ContainerID = mod.ID + if err := b.CreateMicroflow(mf); err != nil { + t.Fatalf("CreateMicroflow on Mendix 9: %v", err) + } + raw, err := b.GetRawUnitBytes(mf.ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + if _, err := bson.Raw(raw).LookupErr("StableId"); err == nil { + t.Errorf("microflow carries StableId, introduced in Mendix 10.2.0") + } + assertPre1015Flows(t, raw, cases) + + // It reads back as written: the case and the curve. + got, err := b.GetMicroflow(mf.ID) + if err != nil { + t.Fatalf("GetMicroflow: %v", err) + } + for _, f := range got.ObjectCollection.Flows { + want, ok := cases[string(f.ID)] + if !ok { + continue + } + ec, isEnum := f.CaseValue.(*microflows.EnumerationCase) + if !isEnum || ec.Value != want { + t.Errorf("flow %s reads back with case %#v, want %q", f.ID, f.CaseValue, want) + } + if want == "true" && (f.OriginControlVector != "15;0" || f.DestinationControlVector != "-30;0") { + t.Errorf("curved flow reads back as %q/%q, want 15;0/-30;0", f.OriginControlVector, f.DestinationControlVector) + } + } + }) + + t.Run("create nanoflow", func(t *testing.T) { + nf := µflows.Nanoflow{Name: "NF_Decide1410", ObjectCollection: decisionFlow()} + nf.ContainerID = mod.ID + if err := b.CreateNanoflow(nf); err != nil { + t.Fatalf("CreateNanoflow on Mendix 9: %v", err) + } + raw, err := b.GetRawUnitBytes(nf.ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + assertPre1015Flows(t, raw, cases) + }) + + t.Run("alter splices a Mendix 9 flow", func(t *testing.T) { + // The splice encodes each new flow with this, and setCase moves the + // case of a stored flow under the key this writes. + d, err := codecMicroflowDeps{b: b}.SerializeSequenceFlow(µflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID("a1410000-0000-0000-0000-000000000021")}, + CaseValue: µflows.EnumerationCase{Value: "true"}, + }) + if err != nil { + t.Fatalf("SerializeSequenceFlow: %v", err) + } + keys := map[string]bool{} + for _, e := range d { + keys[e.Key] = true + } + if keys["CaseValues"] || keys["Line"] || !keys["NewCaseValue"] || !keys["OriginBezierVector"] { + t.Errorf("spliced flow keys = %v, want NewCaseValue and the bezier vectors, no CaseValues/Line", keys) + } + }) +} diff --git a/mdl/backend/modelsdk/flow_version_shape_test.go b/mdl/backend/modelsdk/flow_version_shape_test.go new file mode 100644 index 000000000..23ddde734 --- /dev/null +++ b/mdl/backend/modelsdk/flow_version_shape_test.go @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// flowDocKeys returns the keys of one encoded document. +func flowDocKeys(t *testing.T, raw []byte) map[string]bool { + t.Helper() + elems, err := bson.Raw(raw).Elements() + if err != nil { + t.Fatalf("elements: %v", err) + } + out := map[string]bool{} + for _, e := range elems { + out[e.Key()] = true + } + return out +} + +// TestFlowShapeByVersion pins every version-gated key of the flow writers to +// the Model SDK's floor (mendixlabs/mxcli#1410), across the bands: 9.x, +// 10.0–10.1, 10.2–10.7, 10.8–10.11, 10.12–10.14, 10.15, 10.16, 10.17+. The +// 10.12 row is measured, besides 9.24: Administration.ChangeMyPassword after +// `mx convert` to 10.12.19 stores StableId and ReturnVariableName, a +// NewCaseValue, both bezier vectors and LineType "BezierCurve", and no +// parameter IsRequired/DefaultValue. +func TestFlowShapeByVersion(t *testing.T) { + type want struct { + stableID, returnVar bool + caseList, line, lineType, param bool + } + all := want{stableID: true, returnVar: true, caseList: true, line: true, param: true} + tests := []struct { + v flowVersion + want want + }{ + {flowVersion{9, 24}, want{}}, + {flowVersion{10, 1}, want{}}, + {flowVersion{10, 2}, want{stableID: true}}, + {flowVersion{10, 8}, want{stableID: true, lineType: true}}, + {flowVersion{10, 12}, want{stableID: true, returnVar: true, lineType: true}}, + {flowVersion{10, 14}, want{stableID: true, returnVar: true, lineType: true}}, + {flowVersion{10, 15}, want{stableID: true, returnVar: true, caseList: true, lineType: true}}, + {flowVersion{10, 16}, want{stableID: true, returnVar: true, caseList: true, line: true}}, + {flowVersion{10, 17}, all}, + {flowVersion{11, 6}, all}, + {newestFlowVersion, all}, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("%d.%d", tt.v.major, tt.v.minor), func(t *testing.T) { + mf := µflows.Microflow{ + Name: "MF", + ObjectCollection: decisionFlow(), + Parameters: []*microflows.MicroflowParameter{{Name: "P", Type: µflows.BooleanType{}}}, + } + mf.ObjectCollection.AnnotationFlows = []*microflows.AnnotationFlow{{ + BaseElement: model.BaseElement{ID: "a1410000-0000-0000-0000-000000000031"}, + OriginID: "a1410000-0000-0000-0000-000000000001", + DestinationID: "a1410000-0000-0000-0000-000000000002", + }} + gm := microflowToGen(mf, tt.v) + gm.SetID("a1410000-0000-0000-0000-000000000000") + assignMicroflowIDs(gm) + raw, err := microflowEncoder(tt.v).Encode(gm) + if err != nil { + t.Fatalf("encode: %v", err) + } + check := func(what string, got, want bool) { + t.Helper() + if got != want { + t.Errorf("%s present = %v, want %v", what, got, want) + } + } + top := flowDocKeys(t, raw) + check("Microflow.StableId", top["StableId"], tt.want.stableID) + check("Microflow.ReturnVariableName", top["ReturnVariableName"], tt.want.returnVar) + + flows, _ := bson.Raw(raw).Lookup("Flows").Array().Values() + var seq, ann map[string]bool + for _, f := range flows { + d, ok := f.DocumentOK() + if !ok { + continue + } + switch typ, _ := d.Lookup("$Type").StringValueOK(); typ { + case "Microflows$SequenceFlow": + seq = flowDocKeys(t, d) + case "Microflows$AnnotationFlow": + ann = flowDocKeys(t, d) + } + } + for name, keys := range map[string]map[string]bool{"SequenceFlow": seq, "AnnotationFlow": ann} { + check(name+".Line", keys["Line"], tt.want.line) + check(name+".OriginBezierVector", keys["OriginBezierVector"], !tt.want.line) + check(name+".DestinationBezierVector", keys["DestinationBezierVector"], !tt.want.line) + check(name+".LineType", keys["LineType"], tt.want.lineType) + } + check("SequenceFlow.CaseValues", seq["CaseValues"], tt.want.caseList) + check("SequenceFlow.NewCaseValue", seq["NewCaseValue"], !tt.want.caseList) + check("SequenceFlow.CaseValue (the SDK name, never stored)", seq["CaseValue"], false) + + objs, _ := bson.Raw(raw).Lookup("ObjectCollection", "Objects").Array().Values() + found := false + for _, o := range objs { + d, ok := o.DocumentOK() + if !ok { + continue + } + if typ, _ := d.Lookup("$Type").StringValueOK(); typ == "Microflows$MicroflowParameter" { + found = true + keys := flowDocKeys(t, d) + check("MicroflowParameter.IsRequired", keys["IsRequired"], tt.want.param) + check("MicroflowParameter.DefaultValue", keys["DefaultValue"], tt.want.param) + } + } + if !found { + t.Errorf("no Microflows$MicroflowParameter written") + } + }) + } +} diff --git a/mdl/backend/modelsdk/microflow.go b/mdl/backend/modelsdk/microflow.go index 10f71a71b..f6697429c 100644 --- a/mdl/backend/modelsdk/microflow.go +++ b/mdl/backend/modelsdk/microflow.go @@ -334,6 +334,11 @@ func flowsFromGen(items []element.Element) []*microflows.SequenceFlow { if line, ok := g.Line().(*genMf.BezierCurve); ok && line != nil { f.OriginControlVector = line.OriginControlVector() f.DestinationControlVector = line.DestinationControlVector() + } else { + // Before 10.16 the same two vectors are the flow's own + // Origin/DestinationBezierVector (mendixlabs/mxcli#1410). + f.OriginControlVector = g.OriginBezierVector() + f.DestinationControlVector = g.DestinationBezierVector() } f.ID = model.ID(g.ID()) flows = append(flows, f) diff --git a/mdl/backend/modelsdk/microflow_annotation_test.go b/mdl/backend/modelsdk/microflow_annotation_test.go index 0dd81dc9f..297041f58 100644 --- a/mdl/backend/modelsdk/microflow_annotation_test.go +++ b/mdl/backend/modelsdk/microflow_annotation_test.go @@ -48,7 +48,7 @@ func TestAnnotationFlowToGen(t *testing.T) { OriginID: model.ID("ann-1"), DestinationID: model.ID("act-1"), } - raw, err := (&codec.Encoder{}).Encode(annotationFlowToGen(af, 11)) + raw, err := (&codec.Encoder{}).Encode(annotationFlowToGen(af, newestFlowVersion)) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_idempotence_test.go b/mdl/backend/modelsdk/microflow_idempotence_test.go index 15aecb778..603a9b593 100644 --- a/mdl/backend/modelsdk/microflow_idempotence_test.go +++ b/mdl/backend/modelsdk/microflow_idempotence_test.go @@ -141,7 +141,7 @@ func rebuildMicroflow(t *testing.T, proj string, mf *microflows.Microflow) []byt } defer b.Disconnect() - gm := microflowToGen(mf, b.majorVersion()) + gm := microflowToGen(mf, b.flowVersion()) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) contents, err := (&codec.Encoder{}).Encode(gm) diff --git a/mdl/backend/modelsdk/microflow_mutator_write.go b/mdl/backend/modelsdk/microflow_mutator_write.go index 43ff761ee..5d0dd7f10 100644 --- a/mdl/backend/modelsdk/microflow_mutator_write.go +++ b/mdl/backend/modelsdk/microflow_mutator_write.go @@ -78,19 +78,16 @@ func (d codecMicroflowDeps) SerializeObject(obj microflows.MicroflowObject) (bso } func (d codecMicroflowDeps) SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) { - el := sequenceFlowToGen(f, d.b.majorVersion()) + el := sequenceFlowToGen(f, d.b.flowVersion()) assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } return encodeFlowElementToD(el) } func (d codecMicroflowDeps) SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) { - el := annotationFlowToGen(f, d.b.majorVersion()) + el := annotationFlowToGen(f, d.b.flowVersion()) assignID(el) if af, ok := el.(*genMf.AnnotationFlow); ok { assignID(af.Line()) @@ -105,10 +102,11 @@ func (d codecMicroflowDeps) SerializeDocument(declared any) (bson.D, error) { var el element.Element switch doc := declared.(type) { case *microflows.Microflow: - gm := microflowToGen(doc, d.b.majorVersion()) + fv := d.b.flowVersion() + gm := microflowToGen(doc, fv) gm.SetID(element.ID(doc.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return nil, err } @@ -121,7 +119,7 @@ func (d codecMicroflowDeps) SerializeDocument(declared any) (bson.D, error) { } return out, nil case *microflows.Nanoflow: - g := nanoflowToGen(doc, d.b.majorVersion()) + g := nanoflowToGen(doc, d.b.flowVersion()) g.SetID(element.ID(doc.ID)) assignNanoflowIDs(g) d.b.carryStoredNanoflowHeader(doc, g) diff --git a/mdl/backend/modelsdk/microflow_parameter_position_test.go b/mdl/backend/modelsdk/microflow_parameter_position_test.go index e38901c87..d46ab5781 100644 --- a/mdl/backend/modelsdk/microflow_parameter_position_test.go +++ b/mdl/backend/modelsdk/microflow_parameter_position_test.go @@ -18,18 +18,18 @@ func TestMicroflowParameterToGenKeepsAuthoredPosition(t *testing.T) { Name: "Feedback", Position: &model.Point{X: -77, Y: 0}, } - g := microflowParameterToGen(authored, 0, 11).(*genMf.MicroflowParameter) + g := microflowParameterToGen(authored, 0, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "-77;0" { t.Errorf("authored position = %q, want -77;0", got) } // Control: unannotated parameters still derive from the index. derived := µflows.MicroflowParameter{Name: "Feedback"} - g = microflowParameterToGen(derived, 0, 11).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 0, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "200;53" { t.Errorf("derived position at index 0 = %q, want 200;53", got) } - g = microflowParameterToGen(derived, 1, 11).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 1, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "300;53" { t.Errorf("derived position at index 1 = %q, want 300;53", got) } diff --git a/mdl/backend/modelsdk/microflow_readback.go b/mdl/backend/modelsdk/microflow_readback.go index 10d2bdba7..c7e91cef5 100644 --- a/mdl/backend/modelsdk/microflow_readback.go +++ b/mdl/backend/modelsdk/microflow_readback.go @@ -35,10 +35,11 @@ func (b *Backend) ReadBackMicroflow(mf *microflows.Microflow) (*microflows.Micro return nil, fmt.Errorf("ReadBackMicroflow: nil microflow") } encode := func(m *microflows.Microflow) ([]byte, error) { - gm := microflowToGen(m, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(m, fv) gm.SetID(element.ID(readBackID(m.ID))) assignMicroflowIDs(gm) - raw, err := (&codec.Encoder{}).Encode(gm) + raw, err := microflowEncoder(fv).Encode(gm) return b.completeAsStored(raw), err } written, err := encode(mf) @@ -70,7 +71,7 @@ func (b *Backend) ReadBackNanoflow(nf *microflows.Nanoflow) (*microflows.Nanoflo return nil, fmt.Errorf("ReadBackNanoflow: nil nanoflow") } encode := func(n *microflows.Nanoflow) ([]byte, error) { - g := nanoflowToGen(n, b.majorVersion()) + g := nanoflowToGen(n, b.flowVersion()) g.SetID(element.ID(readBackID(n.ID))) assignNanoflowIDs(g) raw, err := (&codec.Encoder{}).Encode(g) diff --git a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go index cfd8e7a4b..259100bd5 100644 --- a/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go +++ b/mdl/backend/modelsdk/microflow_roundtrip_flags_test.go @@ -18,7 +18,7 @@ import ( // the way out but never read back are silently reset on the return trip. func roundTripMicroflow(t *testing.T, mf *microflows.Microflow) *microflows.Microflow { t.Helper() - gm := microflowToGen(mf, 11) + gm := microflowToGen(mf, newestFlowVersion) raw, err := (&codec.Encoder{}).Encode(gm) if err != nil { t.Fatalf("encode: %v", err) @@ -229,7 +229,7 @@ func TestMicroflowRoundTrip_NoConcurrencyMessageIsInert(t *testing.T) { encMessage := func(t *testing.T, mf *microflows.Microflow) []byte { t.Helper() mf.ID = model.ID("mf-9") - raw, err := (&codec.Encoder{}).Encode(microflowToGen(mf, 11).ConcurrencyErrorMessage()) + raw, err := (&codec.Encoder{}).Encode(microflowToGen(mf, newestFlowVersion).ConcurrencyErrorMessage()) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_rule_split_write_test.go b/mdl/backend/modelsdk/microflow_rule_split_write_test.go index 0e12e17d7..116424cb8 100644 --- a/mdl/backend/modelsdk/microflow_rule_split_write_test.go +++ b/mdl/backend/modelsdk/microflow_rule_split_write_test.go @@ -89,7 +89,7 @@ func TestMicroflowRoundTrip_RuleSplitCondition(t *testing.T) { // agree. Written as "Rule" the reference is invisible to Mendix and the decision is // CE0080 again, for a different reason. Assert the storage key on the raw document. func TestRuleSplitConditionUsesMicroflowStorageKey(t *testing.T) { - raw, err := (&codec.Encoder{}).Encode(microflowToGen(ruleSplitMicroflow(), 11)) + raw, err := (&codec.Encoder{}).Encode(microflowToGen(ruleSplitMicroflow(), newestFlowVersion)) if err != nil { t.Fatalf("encode: %v", err) } diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 11e12fc60..13993fc70 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -143,6 +143,62 @@ func (b *Backend) majorVersion() int { return 11 } +// flowVersion is the Mendix version a flow document — microflow, nanoflow or +// rule — is written for. Flow properties changed shape within Mendix 10, so +// the major alone cannot gate them (mendixlabs/mxcli#1410: every flow write on +// a Mendix 9 project was refused for a StableId and a CaseValues list it +// cannot store, and 10.0–10.14 for the CaseValues list). +// +// The floors are the Model SDK's StructureVersionInfo (mendixmodelsdk +// 4.116.0), and the shapes are measured: a Studio Pro 9.24.30 document, and +// the same document after `mx convert` to 10.12.19. +type flowVersion struct{ major, minor int } + +// newestFlowVersion is what an unreadable project version is written as — +// the newest shape, as majorVersion() has always defaulted to 11. +var newestFlowVersion = flowVersion{major: 11} + +func (v flowVersion) atLeast(major, minor int) bool { + return v.major > major || v.major == major && v.minor >= minor +} + +// Microflow.stableId, introduced in 10.2.0. +func (v flowVersion) hasStableID() bool { return v.atLeast(10, 2) } + +// SequenceFlow.caseValues, introduced in 10.15.0 when the single caseValue +// (stored as NewCaseValue) was deleted. +func (v flowVersion) hasCaseValueList() bool { return v.atLeast(10, 15) } + +// Flow.line, introduced in 10.16.0 when Origin/DestinationBezierVector were +// deleted. Flow.lineType lived between 10.8.0 and 10.16.0. +func (v flowVersion) hasLine() bool { return v.atLeast(10, 16) } +func (v flowVersion) hasLineType() bool { return v.atLeast(10, 8) && !v.hasLine() } + +// MicroflowBase.returnVariableName, introduced in 10.12.0. +func (v flowVersion) hasReturnVariableName() bool { return v.atLeast(10, 12) } + +// MicroflowParameterObject.isRequired / defaultValue, introduced in 10.17.0. +func (v flowVersion) hasParameterDefaults() bool { return v.atLeast(10, 17) } + +// flowVersion returns the version this project's flow documents are written for. +func (b *Backend) flowVersion() flowVersion { + pv := b.ProjectVersion() + if pv == nil || pv.MajorVersion == 0 { + return newestFlowVersion + } + return flowVersion{major: pv.MajorVersion, minor: pv.MinorVersion} +} + +// microflowEncoder encodes a Microflows$Microflow for v. StableId comes from a +// registered Studio Pro default (a fresh GUID), not a gen property, so leaving +// it out below its floor is the encoder's job. +func microflowEncoder(v flowVersion) *codec.Encoder { + if v.hasStableID() { + return &codec.Encoder{} + } + return &codec.Encoder{OmitKeys: map[string]map[string]bool{"Microflows$Microflow": {"StableId": true}}} +} + // CreateMicroflow adds a new microflow document (a top-level unit). func (b *Backend) CreateMicroflow(mf *microflows.Microflow) error { if mf == nil { @@ -154,10 +210,11 @@ func (b *Backend) CreateMicroflow(mf *microflows.Microflow) error { if mf.ID == "" { mf.ID = model.ID(mmpr.GenerateID()) } - gm := microflowToGen(mf, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(mf, fv) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return fmt.Errorf("CreateMicroflow: encode: %w", err) } @@ -175,10 +232,11 @@ func (b *Backend) UpdateMicroflow(mf *microflows.Microflow) error { if b.writer == nil { return fmt.Errorf("UpdateMicroflow: not connected for writing") } - gm := microflowToGen(mf, b.majorVersion()) + fv := b.flowVersion() + gm := microflowToGen(mf, fv) gm.SetID(element.ID(mf.ID)) assignMicroflowIDs(gm) - contents, err := (&codec.Encoder{}).Encode(gm) + contents, err := microflowEncoder(fv).Encode(gm) if err != nil { return fmt.Errorf("UpdateMicroflow: encode: %w", err) } @@ -197,9 +255,10 @@ func (b *Backend) DeleteMicroflow(id model.ID) error { } // microflowToGen builds a gen Microflow wrapper from the model. Mirrors the legacy -// serializer (writer_microflow.go). v10+ adds ReturnVariableName/StableId/Url/ -// UrlSearchParameters. -func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { +// serializer (writer_microflow.go). v10+ adds Url/UrlSearchParameters (the +// property floor drops them empty below 10.1/10.9), 10.12+ ReturnVariableName, +// and 10.2+ StableId (via microflowEncoder). +func microflowToGen(mf *microflows.Microflow, v flowVersion) *genMf.Microflow { out := genMf.NewMicroflow() out.SetName(mf.Name) out.SetDocumentation(mf.Documentation) @@ -239,7 +298,7 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { // Object collection (parameters merged first, then objects). oc := genMf.NewMicroflowObjectCollection() for i, p := range mf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if mf.ObjectCollection != nil { for _, obj := range mf.ObjectCollection.Objects { @@ -254,22 +313,25 @@ func microflowToGen(mf *microflows.Microflow, major int) *genMf.Microflow { // and annotation flows share the same Flows list. if mf.ObjectCollection != nil { for _, f := range mf.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } for _, af := range mf.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } - if major >= 10 { + if v.hasReturnVariableName() { out.SetReturnVariableName(mf.ReturnVariableName) + } + if v.major >= 10 { // Carried, not hardcoded. These two were `""` and `nil` unconditionally, // so every rewrite deleted the microflow's deep link (#1120). A fresh // microflow has neither, so the empty values still come out empty — // SetUrlSearchParametersQualifiedNames(nil) is the empty marker-1 list. out.SetUrl(mf.URL) // StableId is emitted as a fresh GUID binary via the registered default - // (the gen mistypes it as a string), not set here. + // (the gen mistypes it as a string), not set here; microflowEncoder + // leaves it out below 10.2. out.SetUrlSearchParametersQualifiedNames(mf.URLSearchParameters) } return out @@ -421,7 +483,7 @@ func microflowObjectToGen(obj microflows.MicroflowObject) element.Element { // annotationFlowToGen builds a Microflows$AnnotationFlow connecting an Annotation // (origin) to the activity it documents (destination). The line shape is // version-specific, mirroring the legacy serializeAnnotationFlow. -func annotationFlowToGen(af *microflows.AnnotationFlow, major int) element.Element { +func annotationFlowToGen(af *microflows.AnnotationFlow, v flowVersion) element.Element { g := genMf.NewAnnotationFlow() g.SetID(element.ID(af.ID)) g.SetOriginID(element.ID(af.OriginID)) @@ -431,14 +493,19 @@ func annotationFlowToGen(af *microflows.AnnotationFlow, major int) element.Eleme // drew the line out of the top of the note and back down around it. g.SetOriginConnectionIndex(2) g.SetDestinationConnectionIndex(0) - if major <= 9 { - g.SetOriginBezierVector("0;0") - g.SetDestinationBezierVector("0;0") - } else { + // An annotation flow is a Flow like a sequence flow, with the same floors: + // the Line was written from 10.0 on, six minors before it existed. + if v.hasLine() { line := genMf.NewBezierCurve() line.SetOriginControlVector("0;0") line.SetDestinationControlVector("0;0") g.SetLine(line) + } else { + g.SetOriginBezierVector("0;0") + g.SetDestinationBezierVector("0;0") + if v.hasLineType() { + g.SetLineType("BezierCurve") + } } return g } @@ -1444,7 +1511,7 @@ func memberChangeToGen(m *microflows.MemberChange) element.Element { // microflowParameterToGen builds a gen MicroflowParameter. An authored position // is written as given; without one the position derives from the index, matching // the legacy serializer. -func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) element.Element { +func microflowParameterToGen(p *microflows.MicroflowParameter, idx int, v flowVersion) element.Element { g := genMf.NewMicroflowParameter() g.SetID(element.ID(p.ID)) g.SetDocumentation(p.Documentation) @@ -1456,7 +1523,7 @@ func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) e } g.SetRelativeMiddlePoint(fmt.Sprintf("%d;%d", pos.X, pos.Y)) g.SetSize("30;30") - if major >= 10 { + if v.hasParameterDefaults() { g.SetDefaultValue("") g.SetIsRequired(true) } @@ -1464,9 +1531,13 @@ func microflowParameterToGen(p *microflows.MicroflowParameter, idx, major int) e return g } -// sequenceFlowToGen builds a gen SequenceFlow. v10+ uses CaseValues + a BezierCurve -// Line; the case defaults to NoCase. -func sequenceFlowToGen(f *microflows.SequenceFlow, major int) element.Element { +// sequenceFlowToGen builds a gen SequenceFlow in the shape v stores; the case +// defaults to NoCase. Measured shapes (mendixlabs/mxcli#1410): +// +// < 10.15 the case as one NewCaseValue document; 10.15+ a CaseValues list +// < 10.16 Origin/DestinationBezierVector strings (with LineType "BezierCurve" +// from 10.8); 10.16+ a BezierCurve Line holding the control vectors +func sequenceFlowToGen(f *microflows.SequenceFlow, v flowVersion) element.Element { g := genMf.NewSequenceFlow() g.SetID(element.ID(f.ID)) g.SetOriginID(element.ID(f.OriginID)) @@ -1474,14 +1545,26 @@ func sequenceFlowToGen(f *microflows.SequenceFlow, major int) element.Element { g.SetOriginConnectionIndex(int32(f.OriginConnectionIndex)) g.SetDestinationConnectionIndex(int32(f.DestinationConnectionIndex)) g.SetIsErrorHandler(f.IsErrorHandler) - g.AddCaseValues(caseValueToGen(f.CaseValue)) + if v.hasCaseValueList() { + g.AddCaseValues(caseValueToGen(f.CaseValue)) + } else { + g.SetCaseValue(caseValueToGen(f.CaseValue)) + } originCV := orDefault(f.OriginControlVector, "0;0") destCV := orDefault(f.DestinationControlVector, "0;0") - line := genMf.NewBezierCurve() - line.SetOriginControlVector(originCV) - line.SetDestinationControlVector(destCV) - g.SetLine(line) + if v.hasLine() { + line := genMf.NewBezierCurve() + line.SetOriginControlVector(originCV) + line.SetDestinationControlVector(destCV) + g.SetLine(line) + } else { + g.SetOriginBezierVector(originCV) + g.SetDestinationBezierVector(destCV) + if v.hasLineType() { + g.SetLineType("BezierCurve") + } + } return g } @@ -1847,14 +1930,21 @@ func assignMicroflowIDs(m *genMf.Microflow) { for _, el := range m.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } +// assignSequenceFlowIDs assigns IDs to a sequence flow's case and line, in +// whichever shape the flow was built (sequenceFlowToGen). +func assignSequenceFlowIDs(sf *genMf.SequenceFlow) { + assignID(sf.CaseValue()) + for _, cv := range sf.CaseValuesItems() { + assignID(cv) + } + assignID(sf.Line()) +} + // assignObjectCollectionIDs assigns IDs to a collection's objects and their // sub-elements, recursing into loop bodies. func assignObjectCollectionIDs(oc *genMf.MicroflowObjectCollection) { diff --git a/mdl/backend/modelsdk/nanoflow_carry_test.go b/mdl/backend/modelsdk/nanoflow_carry_test.go index 6c8048f49..ea3a72365 100644 --- a/mdl/backend/modelsdk/nanoflow_carry_test.go +++ b/mdl/backend/modelsdk/nanoflow_carry_test.go @@ -88,7 +88,7 @@ func countFlowType(t *testing.T, raw []byte, typeName string) int { func TestNanoflowToGen_WritesAnnotationFlows(t *testing.T) { nf := µflows.Nanoflow{Name: "NF", ObjectCollection: annotatedCollection()} - g := nanoflowToGen(nf, 11) + g := nanoflowToGen(nf, newestFlowVersion) assignNanoflowIDs(g) raw, err := (&codec.Encoder{}).Encode(g) if err != nil { @@ -105,7 +105,7 @@ func TestNanoflowToGen_WritesAnnotationFlows(t *testing.T) { func TestRuleToGen_WritesAnnotationFlows(t *testing.T) { r := µflows.Rule{Name: "R", ReturnType: µflows.BooleanType{}, ObjectCollection: annotatedCollection()} - g := ruleToGen(r, 11) + g := ruleToGen(r, newestFlowVersion) raw, err := (&codec.Encoder{}).Encode(g) if err != nil { t.Fatalf("encode: %v", err) diff --git a/mdl/backend/modelsdk/nanoflow_write.go b/mdl/backend/modelsdk/nanoflow_write.go index e981e8a4e..e1972d09f 100644 --- a/mdl/backend/modelsdk/nanoflow_write.go +++ b/mdl/backend/modelsdk/nanoflow_write.go @@ -29,7 +29,7 @@ func (b *Backend) CreateNanoflow(nf *microflows.Nanoflow) error { if nf.ID == "" { nf.ID = model.ID(mmpr.GenerateID()) } - g := nanoflowToGen(nf, b.majorVersion()) + g := nanoflowToGen(nf, b.flowVersion()) g.SetID(element.ID(nf.ID)) assignNanoflowIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -50,7 +50,7 @@ func (b *Backend) UpdateNanoflow(nf *microflows.Nanoflow) error { if b.writer == nil { return fmt.Errorf("UpdateNanoflow: not connected for writing") } - g := nanoflowToGen(nf, b.majorVersion()) + g := nanoflowToGen(nf, b.flowVersion()) g.SetID(element.ID(nf.ID)) assignNanoflowIDs(g) b.carryStoredNanoflowHeader(nf, g) @@ -76,7 +76,7 @@ func (b *Backend) DeleteNanoflow(id model.ID) error { // serializeNanoflow field set (AllowedModuleRoles, Documentation, Excluded, // Flows, MarkAsUsed, MicroflowReturnType [only when non-void], Name, // ObjectCollection with parameters merged in first). -func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { +func nanoflowToGen(nf *microflows.Nanoflow, v flowVersion) *genMf.Nanoflow { out := genMf.NewNanoflow() out.SetName(nf.Name) out.SetDocumentation(nf.Documentation) @@ -86,14 +86,16 @@ func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { if nf.ReturnType != nil { out.SetMicroflowReturnType(microflowDataTypeToGen(nf.ReturnType)) } - // Only when authored; UpdateNanoflow carries a stored one otherwise. - if nf.ReturnVariableName != "" { + // Only when authored, and only where the version declares it (10.12.0, + // MicroflowBase.returnVariableName); UpdateNanoflow carries a stored one + // otherwise. + if nf.ReturnVariableName != "" && v.hasReturnVariableName() { out.SetReturnVariableName(nf.ReturnVariableName) } oc := genMf.NewMicroflowObjectCollection() for i, p := range nf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if nf.ObjectCollection != nil { for _, obj := range nf.ObjectCollection.Objects { @@ -110,10 +112,10 @@ func nanoflowToGen(nf *microflows.Nanoflow, major int) *genMf.Nanoflow { // (ako/mxcli#705). if nf.ObjectCollection != nil { for _, f := range nf.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } for _, af := range nf.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } return out @@ -172,10 +174,7 @@ func assignNanoflowIDs(n *genMf.Nanoflow) { for _, el := range n.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } diff --git a/mdl/backend/modelsdk/rule_write.go b/mdl/backend/modelsdk/rule_write.go index 468439efb..57f837166 100644 --- a/mdl/backend/modelsdk/rule_write.go +++ b/mdl/backend/modelsdk/rule_write.go @@ -37,7 +37,7 @@ func (b *Backend) CreateRule(rule *microflows.Rule) error { if rule.ID == "" { rule.ID = model.ID(mmpr.GenerateID()) } - g := ruleToGen(rule, b.majorVersion()) + g := ruleToGen(rule, b.flowVersion()) g.SetID(element.ID(rule.ID)) assignRuleIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -58,7 +58,7 @@ func (b *Backend) UpdateRule(rule *microflows.Rule) error { if b.writer == nil { return fmt.Errorf("UpdateRule: not connected for writing") } - g := ruleToGen(rule, b.majorVersion()) + g := ruleToGen(rule, b.flowVersion()) g.SetID(element.ID(rule.ID)) assignRuleIDs(g) contents, err := (&codec.Encoder{}).Encode(g) @@ -96,7 +96,7 @@ func (b *Backend) DeleteRule(id model.ID) error { // pre-7 legacy sibling with nothing to carry through. // - No concurrency group, Url, StableId or action-info slots: those are the // nine properties that make a microflow a microflow. -func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { +func ruleToGen(rule *microflows.Rule, v flowVersion) *genMf.Rule { out := genMf.NewRule() out.SetName(rule.Name) out.SetDocumentation(rule.Documentation) @@ -111,14 +111,16 @@ func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { out.SetExcluded(rule.Excluded) out.SetMarkAsUsed(rule.MarkAsUsed) out.SetApplyEntityAccess(rule.ApplyEntityAccess) - out.SetReturnVariableName(rule.ReturnVariableName) + if v.hasReturnVariableName() { // MicroflowBase.returnVariableName, 10.12.0 + out.SetReturnVariableName(rule.ReturnVariableName) + } if rule.ReturnType != nil { out.SetMicroflowReturnType(microflowDataTypeToGen(rule.ReturnType)) } oc := genMf.NewMicroflowObjectCollection() for i, p := range rule.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, major)) + oc.AddObjects(microflowParameterToGen(p, i, v)) } if rule.ObjectCollection != nil { for _, obj := range rule.ObjectCollection.Objects { @@ -131,12 +133,12 @@ func ruleToGen(rule *microflows.Rule, major int) *genMf.Rule { if rule.ObjectCollection != nil { for _, f := range rule.ObjectCollection.Flows { - out.AddFlows(sequenceFlowToGen(f, major)) + out.AddFlows(sequenceFlowToGen(f, v)) } // Omitted like the nanoflow's, and for the same effect: a rewrite // detached every note (ako/mxcli#705). for _, af := range rule.ObjectCollection.AnnotationFlows { - out.AddFlows(annotationFlowToGen(af, major)) + out.AddFlows(annotationFlowToGen(af, v)) } } return out @@ -154,10 +156,7 @@ func assignRuleIDs(r *genMf.Rule) { for _, el := range r.FlowsItems() { assignID(el) if sf, ok := el.(*genMf.SequenceFlow); ok { - for _, cv := range sf.CaseValuesItems() { - assignID(cv) - } - assignID(sf.Line()) + assignSequenceFlowIDs(sf) } } } diff --git a/mdl/backend/modelsdk/rule_write_test.go b/mdl/backend/modelsdk/rule_write_test.go index 4170ba32b..c751dbeae 100644 --- a/mdl/backend/modelsdk/rule_write_test.go +++ b/mdl/backend/modelsdk/rule_write_test.go @@ -31,7 +31,7 @@ func TestAuthoredRuleMatchesStudioProKeySet(t *testing.T) { ReturnVariableName: "Variable", } - encoded, err := (&codec.Encoder{}).Encode(ruleToGen(rule, 11)) + encoded, err := (&codec.Encoder{}).Encode(ruleToGen(rule, newestFlowVersion)) if err != nil { t.Fatalf("encode rule: %v", err) } @@ -64,7 +64,7 @@ func TestAuthoredRuleSetsExportLevel(t *testing.T) { encoded, err := (&codec.Encoder{}).Encode(ruleToGen(µflows.Rule{ Name: "R", ReturnType: µflows.BooleanType{}, - }, 11)) + }, newestFlowVersion)) if err != nil { t.Fatalf("encode rule: %v", err) } diff --git a/modelsdk/codec/decoder.go b/modelsdk/codec/decoder.go index 8e7bb3f69..7b49f2bba 100644 --- a/modelsdk/codec/decoder.go +++ b/modelsdk/codec/decoder.go @@ -99,6 +99,9 @@ var fieldAliases = map[string]string{ "Type": "NewType", "NewType": "Type", "CaseValue": "NewCaseValue", + // gen now binds the case under its storage name (mendixlabs/mxcli#1410); + // a document holding the SDK name still reads. + "NewCaseValue": "CaseValue", } // DecodeChild decodes a single embedded document child from raw BSON by key. diff --git a/modelsdk/codec/encoder.go b/modelsdk/codec/encoder.go index 836091c24..3154b5567 100644 --- a/modelsdk/codec/encoder.go +++ b/modelsdk/codec/encoder.go @@ -14,7 +14,9 @@ import ( // Encoder serializes Element trees back to BSON bytes. type Encoder struct { - // OmitKeys suppresses BSON keys, per $Type, for this encoder only. + // OmitKeys suppresses BSON keys, per $Type, for this encoder only — a key a + // gen property would emit and a key a registered Studio Pro default would add + // alike (a Microflow's StableId is the latter: mendixlabs/mxcli#1410). // // It exists for **version-floored properties**. Mendix introduces properties // over time, and a key introduced after the project's own version is one that @@ -210,27 +212,27 @@ func (e *Encoder) buildDoc(elem element.Element) (bson.D, error) { } } for _, name := range d.NullFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: nil}) } } for _, name := range d.EmptyStringFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: ""}) } } for _, name := range d.FalseFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: false}) } } for _, name := range d.ZeroGUIDFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: zeroGUIDBinary()}) } } for _, name := range d.FreshGUIDFields { - if !emitted[name] { + if !emitted[name] && !e.omits(elem.TypeName(), name) { doc = append(doc, bson.E{Key: name, Value: idToBinarySubtype0("")}) } } diff --git a/modelsdk/gen/microflows/types.go b/modelsdk/gen/microflows/types.go index 6da016044..1f89209a1 100644 --- a/modelsdk/gen/microflows/types.go +++ b/modelsdk/gen/microflows/types.go @@ -10619,7 +10619,8 @@ func (o *SequenceFlow) InitFromRaw(raw bson.Raw) { if child, err := codec.DecodeChild(raw, "Line"); err == nil { o.line.SetFromDecode(child) } - if child, err := codec.DecodeChild(raw, "CaseValue"); err == nil { + // STORAGE-NAME OVERRIDE, decode half — see initSequenceFlow. + if child, err := codec.DecodeChild(raw, "NewCaseValue"); err == nil { o.caseValue.SetFromDecode(child) } if children, err := codec.DecodeChildren(raw, "CaseValues"); err == nil { @@ -16217,7 +16218,12 @@ func initSequenceFlow() *SequenceFlow { o.lineType.Bind(&o.Base, 6) o.line = property.NewPart[element.Element]("Line") o.line.Bind(&o.Base, 7) - o.caseValue = property.NewPart[element.Element]("CaseValue") + // STORAGE-NAME OVERRIDE: the single case (Model SDK caseValue, deleted in + // 10.15.0 for the caseValues list) is stored under "NewCaseValue" — measured + // on every sequence flow of a Studio Pro 9.24.30 document (mendixlabs/mxcli#1410). + // Bound as "CaseValue" it could be read only through the decoder's alias and + // written under a key no Mendix version declares. + o.caseValue = property.NewPart[element.Element]("NewCaseValue") o.caseValue.Bind(&o.Base, 8) o.caseValues = property.NewPartList[element.Element]("CaseValues") o.caseValues.Bind(&o.Base, 9)