From 73f795ba6374204cebefb08c9b60fd878d95ca30 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:52:56 +0000 Subject: [PATCH 1/4] fix(skills): Vega pack writes JSON numbers locale-independently (#982) formatDecimal(x, '0.00') follows the user's language, so a Dutch user got 12,50 and invalid JSON. Use toString(round(x, 2)), or formatDecimal with a hyphenated locale; explain the silently-ignored underscore tag. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/packs/mendix-vega-charts/SKILL.md | 4 ++-- .claude/skills/packs/mendix-vega-charts/specs/README.md | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.claude/skills/packs/mendix-vega-charts/SKILL.md b/.claude/skills/packs/mendix-vega-charts/SKILL.md index 7f1c356ec4..9d494d0ee0 100644 --- a/.claude/skills/packs/mendix-vega-charts/SKILL.md +++ b/.claude/skills/packs/mendix-vega-charts/SKILL.md @@ -162,12 +162,12 @@ begin set $Json = $Json + $Sep + '{"cat":"' + $R/CategoryName + '"' + ',"m":"' + $Month + '"' - + ',"v":' + formatDecimal($R/Total, '0.00') + '}'; + + ',"v":' + toString(round($R/Total, 2)) + '}'; set $Sep = ','; end loop; ``` -`formatDecimal(x, '0.00')` is the right way to write a number into JSON — it emits a plain decimal with no grouping separators. Never write a value that could be empty into an unquoted position; emit `null` instead, and never emit `0` for "no data" (a zero against a full budget reads as maximally under budget, which is a lie the chart tells convincingly). +`toString(round(x, 2))` is the right way to write a number into JSON: it always uses a `.` decimal point, whatever the user's language, and never switches to an exponent (measured on 11.13: `12.35`, and `0.0000001` stays a plain decimal). **Do not use `formatDecimal(x, '0.00')`** — without a locale argument it formats in the *current user's* language, so a Dutch user gets `12,50` and the chart reports "Data is not valid JSON" while it renders fine for you. If you need `formatDecimal` (fixed trailing zeros), pass an explicit locale with a **hyphenated** tag: `formatDecimal(x, '0.00', 'en-US')`. The underscore form `'nl_NL'`/`'en_US'` is not an error — it is silently ignored and falls back to the user's language, so the bug comes back. Never write a value that could be empty into an unquoted position; emit `null` instead, and never emit `0` for "no data" (a zero against a full budget reads as maximally under budget, which is a lie the chart tells convincingly). ## Verifying without running the app diff --git a/.claude/skills/packs/mendix-vega-charts/specs/README.md b/.claude/skills/packs/mendix-vega-charts/specs/README.md index 2eb3379076..fc3a82f3be 100644 --- a/.claude/skills/packs/mendix-vega-charts/specs/README.md +++ b/.claude/skills/packs/mendix-vega-charts/specs/README.md @@ -41,5 +41,7 @@ out any one and the columns drift apart while every panel insists it is correct. ## Emitting the rows One JSON array of flat objects, built by a microflow over an OQL view entity. Numbers via -`formatDecimal(x, '0.00')`; `null` — never `0` — for "no value"; a discriminator column +`toString(round(x, 2))` — locale-independent, unlike `formatDecimal(x, '0.00')`, which +writes `12,50` for a Dutch user (if you need it, pass a hyphenated locale: +`formatDecimal(x, '0.00', 'en-US')`; `'en_US'` is silently ignored); `null` — never `0` — for "no value"; a discriminator column (`k` above) when one payload feeds several panels. From 997256a6bbf08a32f757f37e659ae9ab87a80cc2 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:52:57 +0000 Subject: [PATCH 2/4] fix(cli): oql and log read the run --local admin port from run-local.json (#982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The run --local hint 'mxcli oql -p …' failed on a non-default --admin-port (cannot connect … localhost:8090), and mxcli log did the same. Both now take the port and password from a live .mxcli/run-local.json for any flag not given. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_log.go | 23 +++-- cmd/mxcli/devloop_admin.go | 44 ++++++++++ cmd/mxcli/devloop_admin_test.go | 147 ++++++++++++++++++++++++++++++++ cmd/mxcli/docker/runlocal.go | 5 +- cmd/mxcli/docker_oql.go | 19 +++-- 5 files changed, 226 insertions(+), 12 deletions(-) create mode 100644 cmd/mxcli/devloop_admin.go create mode 100644 cmd/mxcli/devloop_admin_test.go diff --git a/cmd/mxcli/cmd_log.go b/cmd/mxcli/cmd_log.go index 74ed9f71e2..a8dcd116e3 100644 --- a/cmd/mxcli/cmd_log.go +++ b/cmd/mxcli/cmd_log.go @@ -44,7 +44,10 @@ Levels, most to least severe: NONE CRITICAL ERROR WARNING INFO DEBUG TRACE This talks to the M2EE admin API, so it needs a running app whose admin port is -reachable — typically one started by 'mxcli run --local'. +reachable — typically one started by 'mxcli run --local'. With -p, the admin port +and password that loop recorded in .mxcli/run-local.json are used for any of +--admin-host/--admin-port/--admin-pass not given, so 'run --local --admin-port' +needs no matching flag here. Examples: # What nodes exist, and at what level @@ -177,11 +180,20 @@ func parseLogSetArgs(args []string) ([]docker.LogNodeLevel, error) { return out, nil } +// logAdminOptions builds the admin connection from the flags, taking the port +// and password of a `mxcli run --local` serving -p for any flag not given +// (ako/mxcli#982) — the flag defaults are only right for a loop on 8090. func logAdminOptions(cmd *cobra.Command) docker.M2EEOptions { host, _ := cmd.Flags().GetString("admin-host") port, _ := cmd.Flags().GetInt("admin-port") pass, _ := cmd.Flags().GetString("admin-pass") - return docker.M2EEOptions{Host: host, Port: port, Token: pass, Direct: true} + projectPath, _ := cmd.Flags().GetString("project") + opts := docker.M2EEOptions{Host: host, Port: port, Token: pass, Direct: true} + return devLoopAdminOptions(projectPath, opts, adminFlagsSet{ + host: cmd.Flags().Changed("admin-host"), + port: cmd.Flags().Changed("admin-port"), + token: cmd.Flags().Changed("admin-pass") || os.Getenv("MXCLI_ADMIN_PASS") != "", + }) } // logConnectionHint names the most likely cause when nothing is listening — but @@ -192,11 +204,10 @@ func logConnectionHint(cmd *cobra.Command, err error) string { if !errors.Is(err, docker.ErrAdminUnreachable) { return "" } - host, _ := cmd.Flags().GetString("admin-host") - port, _ := cmd.Flags().GetInt("admin-port") + opts := logAdminOptions(cmd) return fmt.Sprintf(" Log levels come from a RUNNING app's admin API (%s:%d).\n"+ - " Start one with 'mxcli run --local -p ', or point at another with --admin-host/--admin-port/--admin-pass.\n", - host, port) + " Start one with 'mxcli run --local -p ' (and pass the same -p here), or point at another with --admin-host/--admin-port/--admin-pass.\n", + opts.Host, opts.Port) } func init() { diff --git a/cmd/mxcli/devloop_admin.go b/cmd/mxcli/devloop_admin.go new file mode 100644 index 0000000000..2c4f2ade3d --- /dev/null +++ b/cmd/mxcli/devloop_admin.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + +// adminFlagsSet records which admin connection settings the user gave +// explicitly. Those always win; only the rest are taken from a dev loop. +type adminFlagsSet struct { + host, port, token bool +} + +// devLoopAdminOptions points admin-API options at the `mxcli run --local` +// serving projectPath, when one is live and the user did not say otherwise. +// +// The loop records its admin port and password in .mxcli/run-local.json; it is +// the only place a second process can learn them. Without this, `run --local +// --admin-port 8091` printed a `mxcli oql -p …` hint that failed with "cannot +// connect … localhost:8090", and `mxcli log` failed the same way (ako/mxcli#982). +// +// Precedence: explicit flags > live run-local.json > environment > .docker/.env +// > defaults. A handshake whose process is gone is ignored (readDevLoopHandshake +// refuses it), so a crashed loop cannot redirect a query to whatever took its +// port since. A live loop's admin API is loopback HTTP, never docker exec. +func devLoopAdminOptions(projectPath string, opts docker.M2EEOptions, set adminFlagsSet) docker.M2EEOptions { + // An explicit host means "not the loop on this machine"; so do both port + // and password, since nothing would be left to take from the handshake. + if projectPath == "" || set.host || (set.port && set.token) { + return opts + } + hs, err := readDevLoopHandshake(projectPath) + if err != nil || hs.AdminPort == 0 { + return opts + } + if !set.port { + opts.Port = hs.AdminPort + } + if !set.token && hs.AdminPass != "" { + opts.Token = hs.AdminPass + } + opts.Host = "127.0.0.1" + opts.Direct = true + return opts +} diff --git a/cmd/mxcli/devloop_admin_test.go b/cmd/mxcli/devloop_admin_test.go new file mode 100644 index 0000000000..f927d2d246 --- /dev/null +++ b/cmd/mxcli/devloop_admin_test.go @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: Apache-2.0 + +// `mxcli oql` and `mxcli log` find the admin API of a `mxcli run --local` through +// the dev-loop handshake (ako/mxcli#982). Before this, both assumed port 8090, +// so a loop started with --admin-port printed a query hint that failed with +// "cannot connect … localhost:8090". +package main + +import ( + "encoding/base64" + "net" + "net/http" + "net/http/httptest" + "os" + "strconv" + "strings" + "testing" + "time" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + "github.com/spf13/cobra" +) + +// fakeAdminAPI answers the 11.11+ OQL preview route, but only for one password. +func fakeAdminAPI(t *testing.T, pass string) (port int) { + t.Helper() + want := base64.StdEncoding.EncodeToString([]byte(pass)) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-M2EE-Authentication") != want { + w.WriteHeader(http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[{"n":"1"}]}`)) + })) + t.Cleanup(srv.Close) + _, p, err := net.SplitHostPort(srv.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + port, _ = strconv.Atoi(p) + return port +} + +func publishFakeLoop(t *testing.T, project string, port int, pass string) { + t.Helper() + if err := writeDevLoopHandshake(project, devLoopHandshake{ + Project: project, PID: os.Getpid(), AppPort: 18080, + AdminPort: port, AdminPass: pass, Started: time.Now(), + }); err != nil { + t.Fatal(err) + } +} + +// The reported symptom, end to end: the hint `mxcli oql -p ` must reach the +// admin port the loop recorded, with the password it recorded. +func TestOQL_UsesRunLocalAdminPort(t *testing.T) { + t.Setenv("ADMIN_PORT", "") + t.Setenv("M2EE_ADMIN_PASS", "") + p := handshakeProject(t) + port := fakeAdminAPI(t, "loop-secret") + publishFakeLoop(t, p, port, "loop-secret") + + opts := devLoopAdminOptions(p, docker.M2EEOptions{ProjectPath: p}, adminFlagsSet{}) + res, err := docker.ExecuteOQL(docker.OQLOptions{ + Host: opts.Host, Port: opts.Port, Token: opts.Token, + ProjectPath: opts.ProjectPath, Direct: opts.Direct, + }, "SELECT 1") + if err != nil { + t.Fatalf("oql against the run --local admin port: %v", err) + } + if len(res.Rows) != 1 { + t.Fatalf("rows = %d, want 1", len(res.Rows)) + } +} + +// Control: without a handshake the defaults stand, so the test above is +// measuring the handshake and not something else that happens to work. +func TestDevLoopAdminOptions_NoHandshakeKeepsDefaults(t *testing.T) { + p := handshakeProject(t) + in := docker.M2EEOptions{Host: "127.0.0.1", Port: 8090, Token: "x", Direct: true} + got := devLoopAdminOptions(p, in, adminFlagsSet{}) + if got != in { + t.Fatalf("no handshake must leave options alone: got %+v", got) + } +} + +// Explicit flags win over the handshake. +func TestDevLoopAdminOptions_FlagsWin(t *testing.T) { + p := handshakeProject(t) + publishFakeLoop(t, p, 18091, "loop-secret") + in := docker.M2EEOptions{Host: "127.0.0.1", Port: 9999, Token: "mine", Direct: true} + got := devLoopAdminOptions(p, in, adminFlagsSet{port: true, token: true}) + if got.Port != 9999 || got.Token != "mine" { + t.Fatalf("explicit flags overridden: %+v", got) + } + got = devLoopAdminOptions(p, in, adminFlagsSet{}) + if got.Port != 18091 || got.Token != "loop-secret" || !got.Direct { + t.Fatalf("handshake not applied: %+v", got) + } +} + +// A handshake left behind by a dead loop is ignored, not trusted. +func TestDevLoopAdminOptions_StaleHandshakeIgnored(t *testing.T) { + p := handshakeProject(t) + if err := writeDevLoopHandshake(p, devLoopHandshake{PID: 1 << 30, AdminPort: 18091, AdminPass: "s"}); err != nil { + t.Fatal(err) + } + in := docker.M2EEOptions{Port: 8090} + if got := devLoopAdminOptions(p, in, adminFlagsSet{}); got.Port != 8090 { + t.Fatalf("stale handshake used: %+v", got) + } +} + +// `mxcli log` goes through the same resolution: the options it builds from its +// flags must pick up the loop's port when --admin-port was not given. +func TestLogAdminOptions_UsesRunLocalAdminPort(t *testing.T) { + p := handshakeProject(t) + publishFakeLoop(t, p, 18092, "loop-secret") + cmd := newLogFlagsCmd() + if err := cmd.ParseFlags([]string{"-p", p}); err != nil { + t.Fatal(err) + } + got := logAdminOptions(cmd) + if got.Port != 18092 || got.Token != "loop-secret" { + t.Fatalf("log ignores run-local.json: %+v", got) + } + cmd = newLogFlagsCmd() + if err := cmd.ParseFlags([]string{"-p", p, "--admin-port", "9999"}); err != nil { + t.Fatal(err) + } + if got := logAdminOptions(cmd); got.Port != 9999 { + t.Fatalf("--admin-port overridden: %+v", got) + } + if hint := logConnectionHint(cmd, docker.ErrAdminUnreachable); !strings.Contains(hint, ":9999") { + t.Fatalf("hint names the wrong port: %q", hint) + } +} + +func newLogFlagsCmd() *cobra.Command { + c := &cobra.Command{Use: "list"} + c.Flags().StringP("project", "p", "", "") + c.Flags().String("admin-host", "127.0.0.1", "") + c.Flags().Int("admin-port", 8090, "") + c.Flags().String("admin-pass", "mxcli-local-dev", "") + return c +} diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index 4490daec43..1c133e688b 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -825,8 +825,11 @@ func RunLocal(opts LocalRunOptions) error { // The local runtime boots with the live-preview dev flags (see // LocalRuntimeOptions.jvmArgs), so `mxcli oql` can query it directly — and it // now defaults to the local admin password, so no M2EE_ADMIN_PASS is needed - // (findings #36). + // (findings #36). Neither hint names the admin port: both commands read it, + // and the password, from the .mxcli/run-local.json that OnReady just + // published, so a non-default --admin-port needs no flag (ako/mxcli#982). fmt.Fprintf(w, "Query data: mxcli oql -p %s \"SELECT ...\"\n", opts.ProjectPath) + fmt.Fprintf(w, "Log levels: mxcli log list -p %s\n", opts.ProjectPath) if runtimeLog != "" { fmt.Fprintf(w, "Runtime log: %s\n", runtimeLog) } diff --git a/cmd/mxcli/docker_oql.go b/cmd/mxcli/docker_oql.go index 5ac350f6a9..799b410247 100644 --- a/cmd/mxcli/docker_oql.go +++ b/cmd/mxcli/docker_oql.go @@ -20,7 +20,10 @@ By default, when -p is set, the request is routed through "docker compose exec" to reach the container's admin API (which binds to localhost inside the container). Use --direct to bypass docker exec and connect via HTTP directly. -Connection settings are resolved in order: flags > environment variables > .docker/.env > defaults. +Connection settings are resolved in order: flags > the 'mxcli run --local' serving +the project (.mxcli/run-local.json) > environment variables > .docker/.env > defaults. +A live 'run --local' is reached over loopback HTTP on the admin port it recorded, +so a loop started with --admin-port needs no --port here. Examples: # Query with project path (reads .docker/.env for credentials) @@ -42,12 +45,18 @@ Examples: jsonOutput, _ := cmd.Flags().GetBool("json") direct, _ := cmd.Flags().GetBool("direct") + // A `mxcli run --local` serving this project records its admin port and + // password; use them unless the flags say otherwise (ako/mxcli#982). + admin := devLoopAdminOptions(projectPath, + docker.M2EEOptions{Host: host, Port: port, Token: token, ProjectPath: projectPath, Direct: direct}, + adminFlagsSet{host: host != "", port: port != 0, token: token != ""}) + opts := docker.OQLOptions{ - Host: host, - Port: port, - Token: token, + Host: admin.Host, + Port: admin.Port, + Token: admin.Token, ProjectPath: projectPath, - Direct: direct, + Direct: admin.Direct, Stdout: os.Stdout, Stderr: os.Stderr, } From a6a8074c9f7924254f0d36b98b6c8bd7c52ca099 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:53:08 +0000 Subject: [PATCH 3/4] feat(check): MDL-JSONNUM01 notes locale-less formatDecimal in hand-built JSON (#982) Co-Authored-By: Claude Opus 5.5 --- docs-site/src/appendixes/error-messages.md | 21 +++ mdl/executor/validate_json_locale_number.go | 120 ++++++++++++++++++ .../validate_json_locale_number_test.go | 62 +++++++++ mdl/executor/validate_microflow.go | 3 + 4 files changed, 206 insertions(+) create mode 100644 mdl/executor/validate_json_locale_number.go create mode 100644 mdl/executor/validate_json_locale_number_test.go diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index 1662742615..fbecfd008e 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -276,6 +276,27 @@ $n = count $Approved; The same applies to both operands of `union`/`intersect`/`subtract`. +### MDL-JSONNUM01: A locale-dependent number in hand-built JSON + +``` +set '$Json' builds JSON with formatDecimal(…) and no locale: it formats in the user's +language, so a Dutch user gets '12,50' and the JSON is invalid for them only [MDL-JSONNUM01] +``` + +**Cause:** `formatDecimal(x, '0.00')` without a locale argument formats in the *current user's* language. The microflow writes `12.50` for an English user and `12,50` for a Dutch one, so JSON built by concatenation (a chart's data, an API payload) is invalid only for some users — typically not the author. An underscore locale tag (`'nl_NL'`, `'en_US'`) is silently ignored and falls back to the user's language; only the hyphenated form applies. Measured on Mendix 11.13. + +The rule is info and heuristic: it fires when the same `+` concatenation has a string literal containing `{` or `":`. A display string such as `'Total: ' + formatDecimal(…)` is not flagged. + +**Solution:** Use `toString(round(x, 2))`, which always writes a `.` and never an exponent, or pass a hyphenated locale. + +```text +-- WRONG +set $Json = $Json + ',"v":' + formatDecimal($R/Total, '0.00') + '}'; +-- RIGHT +set $Json = $Json + ',"v":' + toString(round($R/Total, 2)) + '}'; +set $Json = $Json + ',"v":' + formatDecimal($R/Total, '0.00', 'en-US') + '}'; +``` + ### Mismatched input ``` diff --git a/mdl/executor/validate_json_locale_number.go b/mdl/executor/validate_json_locale_number.go new file mode 100644 index 0000000000..3213c082c8 --- /dev/null +++ b/mdl/executor/validate_json_locale_number.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// checkLocaleNumberInJSON flags formatDecimal without a locale argument inside a +// string concatenation that builds JSON — MDL-JSONNUM01, ako/mxcli#982. +// +// formatDecimal(x, '0.00') formats in the CURRENT USER's language, so the same +// microflow writes `12.50` for an English user and `12,50` for a Dutch one, and +// the JSON is invalid only for the second ("Data is not valid JSON" in a chart +// widget that works for its author). Measured on 11.13: +// +// formatDecimal(1234.5, '0.00', 'nl-NL') → 1234,50 +// formatDecimal(1234.5, '0.00', 'nl_NL') → the user's default: the underscore +// tag is silently ignored +// toString(round(0.0000001, 2)) → a plain decimal, no exponent +// +// It is a heuristic, so it is info: "builds JSON" means the same `+` chain has a +// string literal containing `{` or `":`, which is how a hand-built JSON object +// looks and an ordinary display string ("Total: " + …) does not. A third +// argument of any value is accepted; checking the tag's spelling is left to the +// suggestion. +func (v *microflowValidator) checkLocaleNumberInJSON(label string, expr ast.Expression) { + if expr == nil { + return + } + reported := false + var walk func(e ast.Expression, inChain bool) + walk = func(e ast.Expression, inChain bool) { + switch n := e.(type) { + case *ast.SourceExpr: + walk(n.Expression, inChain) + case *ast.ParenExpr: + walk(n.Inner, false) + case *ast.BinaryExpr: + if n.Operator == "+" && !inChain && !reported { + var parts []ast.Expression + flattenConcat(n, &parts) + if call := localelessFormatDecimalInJSON(parts); call != nil { + reported = true + v.addViolation("MDL-JSONNUM01", linter.SeverityInfo, + fmt.Sprintf("%s builds JSON with formatDecimal(…) and no locale: it formats in the "+ + "user's language, so a Dutch user gets '12,50' and the JSON is invalid for them only", label), + "Use toString(round(x, 2)) — locale-independent, no exponent — or pass a hyphenated "+ + "locale: formatDecimal(x, '0.00', 'en-US'). An underscore tag ('en_US') is silently ignored.") + } + } + isPlus := n.Operator == "+" + walk(n.Left, isPlus) + walk(n.Right, isPlus) + case *ast.UnaryExpr: + walk(n.Operand, false) + case *ast.FunctionCallExpr: + for _, a := range n.Arguments { + walk(a, false) + } + case *ast.IfThenElseExpr: + walk(n.Condition, false) + walk(n.ThenExpr, false) + walk(n.ElseExpr, false) + } + } + walk(expr, false) +} + +// flattenConcat collects the operands of a `+` chain, looking through the +// SourceExpr wrapper but not through parentheses (a parenthesised `+` may be +// arithmetic). +func flattenConcat(e ast.Expression, out *[]ast.Expression) { + switch n := e.(type) { + case *ast.SourceExpr: + flattenConcat(n.Expression, out) + case *ast.BinaryExpr: + if n.Operator == "+" { + flattenConcat(n.Left, out) + flattenConcat(n.Right, out) + return + } + *out = append(*out, e) + default: + *out = append(*out, e) + } +} + +func localelessFormatDecimalInJSON(parts []ast.Expression) *ast.FunctionCallExpr { + looksJSON := false + var call *ast.FunctionCallExpr + for _, p := range parts { + for { + if s, ok := p.(*ast.SourceExpr); ok { + p = s.Expression + continue + } + break + } + switch n := p.(type) { + case *ast.LiteralExpr: + if s, ok := n.Value.(string); ok && n.Kind == ast.LiteralString && + (strings.Contains(s, "{") || strings.Contains(s, `":`)) { + looksJSON = true + } + case *ast.FunctionCallExpr: + if strings.EqualFold(n.Name, "formatDecimal") && len(n.Arguments) < 3 && call == nil { + call = n + } + } + } + if looksJSON { + return call + } + return nil +} diff --git a/mdl/executor/validate_json_locale_number_test.go b/mdl/executor/validate_json_locale_number_test.go new file mode 100644 index 0000000000..d2f060896e --- /dev/null +++ b/mdl/executor/validate_json_locale_number_test.go @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// jsonNumViolations parses real MDL and returns the MDL-JSONNUM01 hits, going +// through the visitor so the expression shape is the one users actually get. +func jsonNumViolations(t *testing.T, body string) int { + t.Helper() + prog := parseMDL(t, `create or replace microflow Shop.Build($Total: Decimal) returns String +begin + declare $Json String = ''; + `+body+` + return $Json; +end;`) + n := 0 + for _, stmt := range prog.Statements { + mf, ok := stmt.(*ast.CreateMicroflowStmt) + if !ok { + continue + } + for _, v := range ValidateMicroflow(mf) { + if v.RuleID == "MDL-JSONNUM01" { + n++ + } + } + } + return n +} + +// The skill's old idiom (ako/mxcli#982): JSON built by concatenation with a +// locale-less formatDecimal, which writes '12,50' for a Dutch user. +func TestJSONNum_LocalelessFormatDecimalInJSON(t *testing.T) { + for name, body := range map[string]string{ + "object key": `set $Json = $Json + ',"v":' + formatDecimal($Total, '0.00') + '}';`, + "brace only": `set $Json = '{' + formatDecimal($Total, '0.00') + '}';`, + "in declare": `declare $J String = '{"v":' + formatDecimal($Total, '0.00') + '}';`, + } { + if got := jsonNumViolations(t, body); got != 1 { + t.Errorf("%s: %d MDL-JSONNUM01, want 1", name, got) + } + } +} + +// Controls: the fixes, and the same call outside JSON, are not flagged. +func TestJSONNum_NotFlagged(t *testing.T) { + for name, body := range map[string]string{ + "toString(round)": `set $Json = $Json + ',"v":' + toString(round($Total, 2)) + '}';`, + "explicit locale": `set $Json = $Json + ',"v":' + formatDecimal($Total, '0.00', 'en-US') + '}';`, + "display string": `set $Json = 'Total: ' + formatDecimal($Total, '0.00');`, + "no concatenation": `set $Json = formatDecimal($Total, '0.00');`, + } { + if got := jsonNumViolations(t, body); got != 0 { + t.Errorf("%s: %d MDL-JSONNUM01, want 0", name, got) + } + } +} diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index b8c1d24932..3fdf5fd418 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -556,6 +556,9 @@ func (v *microflowValidator) checkStmtExprFunctions(s ast.MicroflowStatement) { // check but fail the build with CE0117. label describes where the expression // appears (e.g. "declare '$r'"). (findings #1) func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expression) { + // The one per-expression hook shared by microflows and nanoflows, so the + // JSON-number check rides along rather than keeping a third list of sites. + v.checkLocaleNumberInJSON(label, expr) src := microflowExprSource(expr) if src == "" { return From ba8406025ab04e9848ac8b7d0b031e9a86ebba8f Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:53:09 +0000 Subject: [PATCH 4/4] docs: changelog and findings for #982 Co-Authored-By: Claude Opus 5.5 --- .claude/skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + .claude/skills/fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 3 +++ 3 files changed, 5 insertions(+) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 76b151ce58..89be86dc8f 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -161,3 +161,4 @@ {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch` (Mendix <= 11.13, rollup bundler): after a while every page change fails with `web client rebuild failed: web client watcher exited` (or `client bundle not served after apply: web client re-bundle: web client build timed out after 5m0s`) and nothing reaches the browser until `run --local` is restarted; after adding an entity the next changes fail with `ENOTDIR: not a directory, stat '.../web/pages/.js/package.json'`", "cause": "watchAndApply held a bare *WebClientWatcher: (1) once it exited nothing restarted it (only recoverMissingPages did), so WaitForRebuild failed every later change; (2) a failed incremental build (rollup's commonjs resolver hitting web/pages mid-rewrite by the serve build) left the watcher erroring and the change was dropped; (3) ensureClientServed's recovery ran a one-shot NODE_ENV=production BuildWebClient in the same web/ dir while the watcher was still running — two rollups on web/dist; (4) the 5m limit was hard-coded and a timeout printed nothing about why", "file": "cmd/mxcli/docker/webclient_supervisor.go, cmd/mxcli/docker/runlocal.go (watchAndApply, ensureClientServed), cmd/mxcli/docker/webclient.go (webClientTimeout, webClientBuildLogTail)", "fix": "bundlerSupervisor owns the bundler: EnsureAlive restarts an exited one (backoff 2s..60s after failed starts), AwaitRebuild retries a failed/aborted incremental rebuild once with a fresh bundler, Rebundle = stop+reap then start (never two). ensureClientServed takes the rebundle func; clientRebundler hands it the supervisor under --watch and the one-shot otherwise. --web-client-timeout / MXCLI_WEB_CLIENT_TIMEOUT; timeout errors append the last 30 lines of deployment/log/web-client-build.log. sessionNotice prints that a restart dropped sessions", "insight": "Killing the runner (`kill `) reproduces the dead-watcher state in seconds — no need to wait for it to die on its own. A fresh bundler is the universal recovery under --watch: its first build is a full bundle of the current source, so it covers missing pages, dangling chunks, a dist/ deleted by Gradle, and a transient incremental failure alike, without a second process on web/dist. Note: exec.Cmd.Wait called a second time concurrently with the reaper did block until exit on this Go version, so the old Stop was not the overlap — the one-shot in ensureClientServed was", "test": "cmd/mxcli/docker/webclient_supervisor_test.go (TestBundlerSupervisor_RestartsExitedBundler, _AwaitRebuildRecovers, _RebundleNeverOverlaps, TestEnsureClientServed_WatchModeRebundleIsExclusive, TestBuildWebClient_TimeoutShowsLogTail); live: 11.13 scratch app, 7 consecutive changes incl. a killed runner"} {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch`: an `mxcli exec` (or save) made while a change is still building/applying is never built — no `Change detected` follows, the app keeps the previous model, and re-running the script writes nothing (byte-idempotent) so nothing re-triggers", "cause": "watchAndApply set `last = sourceMTime(...)` after every successful apply, under a comment claiming it kept mid-build edits; it did the opposite — the edit's mtime was folded into the baseline, so the next tick saw nothing newer", "file": "cmd/mxcli/docker/runlocal.go (watchAndApply)", "fix": "keep `last` at the settled mtime the build was taken from; the build writes nothing under the watched model/theme source (checked with find -newer during a live run), so this cannot self-trigger", "insight": "Found while reproducing #971 with a script that waited for the first output line of a change instead of its `applied` line — the next exec landed during a 2-minute restart-apply and vanished. Any test of a watch loop should include an edit made DURING a build, not only between builds", "test": "live only (11.13 scratch app, hsqldb): exec an entity add, exec a page change 15s into its build; fixed binary builds it as the next build, the binary with the refresh restored shows no further build after 45s"} {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "ako/mxcli#970 item 2: `theme create --from design.css` with only a :root block (no dark block) wrote the design's light --mxt-ground/--mxt-ink/--mxt-brand into the base theme's dark mixin, whose other surfaces stayed dark; the first injected token also sat on the `@mixin … {` line, unindented", "cause": "Tokens.forVariant returned the base declarations for EITHER variant, and seedTokens applied it to the alt-palette mixin unconditionally; applyTokens matched `(?m)^(\\s*)name`, and \\s* at ^ swallows the preceding newline (and blank line), so the replacement lost its line break and indent", "file": "`cmd/mxcli/theme/create.go` (seedTokens, Create/CreateResult.UnseededVariant); `cmd/mxcli/theme/tokens.go` (applyTokens, Tokens.declares); `cmd/mxcli/cmd_theme.go` (note)", "fix": "seed the alt mixin only when the design declared a block for that variant; otherwise leave it byte-identical to the base and report UnseededVariant, which the CLI prints as a note; match the indent with [ \\t]* instead of \\s*", "insight": "A base palette is the default variant's palette, not 'both': a token set that does not say which variant it describes must not seed the other one. And under (?m), ^\\s* is not 'leading indentation' — it crosses lines; use [ \\t]*. The control for 'mixin untouched' is the same scaffold with no design at all, compared byte for byte", "test": "`cmd/mxcli/theme/create_variant_test.go` (all three bases, base-only vs variant block, indentation); `cmd/mxcli/cmd_theme_test.go` (TestThemeCreate_NotesTheVariantABaseOnlyDesignDidNotSeed)"} +{"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "ako/mxcli#982 item 2: `run --local --admin-port 8091` printed `Query data: mxcli oql -p app.mpr` and that command failed `cannot connect to Mendix admin API at localhost:8090`; `mxcli log list` failed the same way", "cause": "oql resolved the admin port as flag > ADMIN_PORT env > .docker/.env > 8090 and log used flag defaults 8090/mxcli-local-dev; neither read the .mxcli/run-local.json handshake the loop publishes with its port and password (only `constant set --apply` did)", "file": "`cmd/mxcli/devloop_admin.go` (devLoopAdminOptions); `cmd/mxcli/docker_oql.go`; `cmd/mxcli/cmd_log.go` (logAdminOptions, logConnectionHint); `cmd/mxcli/docker/runlocal.go` (hint)", "fix": "one helper takes port/password from a LIVE handshake for any flag not given (explicit host means 'not this loop'), forces direct loopback HTTP; oql and log both use it; the log hint prints the resolved port", "insight": "A hint printed by the process that knows the port must be runnable without that knowledge: either print the flags or make the consumer read what the producer published. Reading the handshake fixes every consumer at once; the test is an httptest admin API on a random port with a fake run-local.json, controlled by the no-handshake and stale-pid cases", "test": "`cmd/mxcli/devloop_admin_test.go`"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 7723d68b23..35f1887a25 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -857,3 +857,4 @@ {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#968 / mendixlabs/mxcli#1263: `datepicker d (DateFormat: Time)` or `DateFormat: Custom, CustomDateFormat: '\u2026'` passes check and exec, mx check 0 errors, but every picker is stored FormattingInfo.DateFormat=Date; describe prints no format, so describe \u2192 exec silently turns a Studio Pro date-time picker into a date-only one. Same drop for a text box's DecimalPrecision/GroupDigits", "cause": "Three hops each dropped it: buildDatePickerV3/buildTextBoxV3 never read the properties, widget_write.go hard-coded newFormattingInfo() on DatePicker and TextBox, and describe never extracted FormattingInfo. Check stayed silent because validateStaticWidgetUnknownProps exempted the dynamic-text format keys (dateformat, customdateformat, \u2026) on EVERY widget type, not just dynamictext", "fix": "pages.DatePicker.FormattingInfo; executor input_formatting.go (inputFormattingInfo + inputFormattingProblems shared by builder and MDL-WIDGET18 check), writer formattingInfoToGen(x.FormattingInfo), describeInputFormatting, pagemutator setWidgetFormattingMut; per-widget allow-list pages.FormattingProperties. Measured: Custom with empty pattern = CE0493; Studio Pro stores CustomDateFormat beside DateFormat DateTime (TestApp WorkflowCommons), so only a pattern with NO DateFormat is refused \u2014 the param-format rule that refused it broke check on describe output", "file": "mdl/executor/input_formatting.go", "insight": "A key exempted from the unknown-property warning must be exempted per widget type: the dynamic-text format keys were skipped on every widget, which turned `DateFormat:` on a date picker (where nothing read it) into a silent drop. Before refusing a cross-field combination, scan Studio Pro-authored units for it \u2014 CustomDateFormat beside DateTime is stored by Studio Pro, and refusing it broke check on describe output.", "test": "mdl/executor/input_formatting_pedapp_test.go, input_formatting_test.go, mdl/backend/modelsdk/widget_formatting_write_test.go"} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 2: `alter page … { set Action = microflow M.X on btn }` with M.X created earlier in the same script failed check (\"microflow not found\") and exec refused the script; the same for nanoflow and show page targets", "cause": "validateAlterSetProperties dry-runs the SET against the stored document, and resolveMicroflow / resolveNanoflowByName / resolvePageRef only know the session cache (createdMicroflows, …) that executing fills — which check never does", "file": "`mdl/executor/validate_alter_set.go` (scriptDeclaresMissing)", "insight": "A dry run of a mutator in check must treat a NotFound for a name the script declares (scriptContext.microflows/nanoflows/pages/snippets) as satisfied, matching on the typed mdlerrors.NotFoundError Kind+Name through errors.As rather than the message. Do not register fake IDs in ctx.Cache instead: exec runs check on the same executor and would resolve to them. Control: an undeclared target still fails", "refs": ["#969"]} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 3: a list view / data grid / gallery with `datasource: $currentObject/M.Assoc` over a single-object association passed check and exec, then mxbuild failed CE8812 \"A grid association path must result in a list\"", "cause": "No rule modelled association multiplicity for list widgets", "file": "`mdl/executor/validate_assoc_list_source.go` (MDL-ASSOCDS01), hooked into attributeScopeValidator.walk", "insight": "Measured 8 shapes x 3 widgets on 11.13.0 and 11.14.0, identical: CE8812 for a Reference followed from its FROM entity (owner Default or Both) and for a Reference with owner Both from the TO entity (one-to-one); the reverse of a default Reference and every ReferenceSet build clean. Judge only those shapes with an exact context entity; skip specializations, self-associations and multi-hop paths. The attribute-scope walk already carries the data context, so hook there", "refs": ["#969"]} +{"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#982 item 1: a chart microflow built JSON with `',\"v\":' + formatDecimal($x, '0.00')` (as the Vega skill pack recommended); a Dutch user got `12,50` and 'Data is not valid JSON', while it worked for the author", "cause": "formatDecimal without a locale formats in the current user's language; an underscore tag ('nl_NL') is silently ignored, only hyphenated tags ('en-US') apply. toString(round(x, 2)) is locale-independent with no exponent (measured 11.13)", "file": "`.claude/skills/packs/mendix-vega-charts/SKILL.md`, `specs/README.md`; `mdl/executor/validate_json_locale_number.go` (MDL-JSONNUM01, hooked in checkExprFunctions)", "fix": "skill uses toString(round(x, 2)) and explains the locale trap; check emits info MDL-JSONNUM01 for a locale-less formatDecimal in a + chain whose string literals contain '{' or '\":'", "insight": "Locale-dependent output passes every test run by the author, whose language is the one that works; the measurement that settles it is the same call under a second locale. The lint heuristic keys on the JSON-looking literal in the same concatenation so display strings stay quiet", "test": "`mdl/executor/validate_json_locale_number_test.go` (positive shapes + controls: toString(round), explicit locale, display string, bare call)"} diff --git a/CHANGELOG.md b/CHANGELOG.md index f408007929..d1b972b27f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`mxcli oql -p` and `mxcli log` reach a `run --local` started with `--admin-port`** (ako/mxcli#982) — both read the admin port and password the loop records in `.mxcli/run-local.json` for any connection flag not given, so the `Query data: mxcli oql -p …` hint `run --local` prints works on a non-default port instead of failing "cannot connect … localhost:8090". `run --local` now also prints a `mxcli log list -p …` hint. Precedence: flags, then a live loop, then environment, `.docker/.env`, defaults; a handshake left by a stopped loop is ignored. +- **The Vega charts skill pack writes JSON numbers with `toString(round(x, 2))`** (ako/mxcli#982) — it recommended `formatDecimal(x, '0.00')`, which follows the user's language: a Dutch user got `12,50` and "Data is not valid JSON". Measured on 11.13: `toString(round(…))` is locale-independent and has no exponent; `formatDecimal(x, '0.00', 'en-US')` also works, but an underscore tag (`'en_US'`) is silently ignored. - **`run --local --watch` no longer loses an edit made while a change is being applied** — after each apply the loop moved its change baseline to the source's current mtime, so an `exec` or save that landed during the build (easily, while a structural change restarts the runtime) was never rebuilt: the app kept serving the previous model with nothing reported. The baseline now stays at the mtime the build was taken from, and the edit is built on the next poll. - **`run --local --watch` keeps its web client bundler alive and never runs two** (ako/mxcli#971) — a bundler that exited stayed dead, so every later page change failed with `web client watcher exited` and only restarting `run --local` recovered (reproduced on Mendix 11.13 by killing the runner: two changes in a row failed). It is now restarted on the next change, with backoff when it cannot start. An incremental rebuild that fails — measured after adding an entity: `ENOTDIR … web/pages/.js/package.json`, and the next change failed the same way — is retried once with a fresh bundler. A recovery re-bundle (a missing page, a dangling chunk, or `/dist/index.js` gone after a runtime restart) replaces the bundler instead of running a one-shot production bundle beside it in the same `web/` directory. The bundle-build limit is configurable (`--web-client-timeout`, or `MXCLI_WEB_CLIENT_TIMEOUT`; default 5m), and a timeout prints the tail of `deployment/log/web-client-build.log`. A change that restarts the runtime now says that browser sessions were dropped. - **`theme create --from` no longer paints a dark palette with a design's light colours** (ako/mxcli#970) — a design that declares only a base palette describes one variant, but its values were also written into the base theme's alternate-variant mixin: `--mxt-ground: #f7f9fb` and `--mxt-ink: #1b2733` landed in the dark mixin while its surfaces stayed dark, an unreadable mix. With no block for that variant the mixin is now left exactly as the base theme ships it, and `create` prints a note naming it and how to seed it (a `prefers-color-scheme: dark` block, or `light` for a dark-first base). Also fixed: the first rewritten token of a block landed on the `@mixin … {` line, unindented, and a rewritten token after a blank line swallowed the blank line. Verified with mxbuild 11.14.0's bundled sass on signal, console and ledger, with and without a variant block. @@ -146,6 +148,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **`check` notes a locale-less `formatDecimal` in hand-built JSON** (ako/mxcli#982) — **MDL-JSONNUM01** (info) for `formatDecimal(x, '0.00')` inside a `+` concatenation whose string literals contain `{` or `":`, which writes `12,50` for a Dutch user. Use `toString(round(x, 2))` or pass a hyphenated locale. A display string (`'Total: ' + formatDecimal(…)`) is not flagged. - **`mxcli fix hashes` verifies and repairs the MPR v2 ContentsHash index** (ako/mxcli#972) — every `Unit.ContentsHash` in the `.mpr` is compared with `base64(SHA-256)` of its `.mxunit`, and mismatches, missing files and orphan files are reported; the command exits 1 when any remain. `--repair` rewrites the mismatched hashes from the files in one transaction, under the same Studio Pro open-project guard as every write. Restoring a unit with `git checkout` leaves the index stale and `mx check` does not notice; measured on a TestApp copy, an entity added by `exec` and then reverted with a byte-level restore is reported as one `DomainModels$DomainModel` mismatch, and after `--repair` the verify is clean and `mx check` reports 0 errors. `exec` and `docker check` print a one-line warning when the index disagrees (about 0.2 s on 900 units). An MPR v1 project has no index, and the command says so. - **Warnings for git states that crash Studio Pro** (ako/mxcli#972) — Studio Pro 11.13 fails to open a project ("Unable to find 'system' property in 'system'") when its git branch has no upstream or git reports "detected dubious ownership". `docker check`, `run --local` and the new `mxcli diag -p app.mpr` project section warn with the remedy (`git push -u origin `; `git config --global --add safe.directory ` on the Studio Pro machine). Never fatal, and silent outside a git repository, on a detached HEAD, under `CI`, or with `MXCLI_NO_GIT_WARNINGS=1`. See the new "Working Outside Studio Pro" page. - **`create translations … without marketplace`, and an unscoped translations run warns when it writes into Marketplace modules** (ako/mxcli#970) — without `in `, `create [or modify|or replace] translations` reaches the whole project, Marketplace modules and their Atlas page templates and building blocks included; on TestApp `'Cancel' as 'Annuleren'` changed 35 Marketplace documents of 38. A module update replaces those modules, so the translations are lost at the next update and show up as unexpected diffs until then. The run now warns with the count per module and how many are templates or building blocks. `without marketplace` (also on `describe translations`, which emits it) keeps the run, and an `or replace` deletion, to the app's own modules and names the file's entries it left alone. The default is unchanged: what an existing script writes does not change underneath it (ADR-0011).