From 66062abb9012eb0a332279b49de0a767999621cc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 14:51:27 +0000 Subject: [PATCH 1/5] feat(init): seed a lint config that excludes the System module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mxcli lint` on a freshly initialised app reported ~106 issues, of which ~102 came from the System module — Mendix's own platform module, whose entities you cannot document, give access rules, or rename members of. Every one of those findings is un-actionable, and they buried the handful about the developer's own code. Measured on a real project here: 170 issues, 102 of them System, across QUAL002 (50), SEC001 (38), CONV001 (8), DESIGN001 (4), MPR003 and SEC006. `mxcli lint` already had -e/--exclude and a lint-config.yaml with excludeModules, so this was solvable — but only by a user who already knew to look. The default is what people see. `mxcli init` now writes .claude/lint-config.yaml with System excluded, which drops that project from 170 findings to 68. The file is written only when the project has no lint config in any location FindConfigFile searches, so re-running init never discards edits. It is written outside the per-tool branches because `mxcli lint` reads it regardless of which AI tool was selected. Also warns when --modules names a config-excluded module. LintContext .IsExcluded checks the exclude set before the include set, so `lint -m System` against the new default would otherwise report zero findings with no explanation — a trap this change would have introduced. Addresses issuetracker finding #9. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- README.md | 2 +- cmd/mxcli/cmd_lint.go | 50 +++++++++++++ cmd/mxcli/init.go | 64 ++++++++++++++++ cmd/mxcli/init_lint_config_test.go | 116 +++++++++++++++++++++++++++++ docs-site/src/ide/init-output.md | 21 +++++- 5 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 cmd/mxcli/init_lint_config_test.go diff --git a/README.md b/README.md index 8e750c9c2..18de40a9e 100644 --- a/README.md +++ b/README.md @@ -498,7 +498,7 @@ mxcli add-tool cursor - `.ai-context/examples/` - Example MDL scripts **Tool-Specific:** -- **Claude Code**: `.claude/settings.json`, `CLAUDE.md`, commands, lint-rules, skills +- **Claude Code**: `.claude/settings.json`, `CLAUDE.md`, commands, lint-rules, `lint-config.yaml` (System module excluded from lint), skills - **Cursor**: `.cursorrules` - Compact MDL reference - **Continue.dev**: `.continue/config.json` - Custom commands and slash commands - **Windsurf**: `.windsurfrules` - MDL rules for Codeium diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go index ac9229c0e..b984463e1 100644 --- a/cmd/mxcli/cmd_lint.go +++ b/cmd/mxcli/cmd_lint.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/linter/rules" @@ -186,6 +187,20 @@ Examples: if len(cfg.ExcludeModules) > 0 { merged := append(excludeModules, cfg.ExcludeModules...) ctx.SetExcludedModules(merged) + // An exclude always wins over --modules (LintContext.IsExcluded + // checks the exclude set first), so asking for a module the + // config excludes yields zero findings with no explanation. + // `mxcli init` now ships a config excluding System, which makes + // `lint -m System` exactly that trap — say so rather than + // returning a silent empty result. + if shadowed := intersect(moduleFilter, cfg.ExcludeModules); len(shadowed) > 0 { + fmt.Fprintf(os.Stderr, + "Warning: --modules names %s, but %s excluded by %s — no findings will be reported for %s. Remove it from excludeModules to lint it.\n", + strings.Join(shadowed, ", "), + pluralIsAre(len(shadowed)), + configPath, + pluralItThem(len(shadowed))) + } } cfg.ApplyConfig(lint) } else { @@ -252,3 +267,38 @@ func catalogRefreshCommand(mode linter.CatalogMode) string { return "REFRESH CATALOG" } } + +// intersect returns the values of want that appear in have, preserving want's +// order and dropping duplicates. +func intersect(want, have []string) []string { + if len(want) == 0 || len(have) == 0 { + return nil + } + inHave := make(map[string]bool, len(have)) + for _, h := range have { + inHave[h] = true + } + seen := make(map[string]bool, len(want)) + var out []string + for _, w := range want { + if inHave[w] && !seen[w] { + seen[w] = true + out = append(out, w) + } + } + return out +} + +func pluralIsAre(n int) string { + if n == 1 { + return "it is" + } + return "they are" +} + +func pluralItThem(n int) string { + if n == 1 { + return "it" + } + return "them" +} diff --git a/cmd/mxcli/init.go b/cmd/mxcli/init.go index df8c83281..fdb1fb51b 100644 --- a/cmd/mxcli/init.go +++ b/cmd/mxcli/init.go @@ -12,6 +12,7 @@ import ( "slices" "strings" + "github.com/mendixlabs/mxcli/mdl/linter" "github.com/spf13/cobra" ) @@ -213,6 +214,12 @@ Container Runtime: } } + // Seed the lint config. `mxcli lint` reads this regardless of which AI + // tool was selected, so it is written outside the per-tool branches. + if path, created := writeDefaultLintConfig(absDir); created { + fmt.Printf(" Created %s (System module excluded from lint)\n", filepath.Base(path)) + } + // Write universal skills to .ai-context/skills/ skillCount := 0 err = fs.WalkDir(skillsFS, "skills", func(path string, d fs.DirEntry, err error) error { @@ -579,6 +586,63 @@ Container Runtime: }, } +// defaultLintConfig is the lint configuration written into a freshly +// initialised project. System is excluded because its contents are Mendix's, +// not the developer's: you cannot document its entities, give them access +// rules, or rename their members. Linting it produced ~100 un-actionable +// findings on a blank app, which buried the handful about the developer's own +// code (issuetracker finding #9). +const defaultLintConfig = `# mxcli lint configuration. +# Docs: mxcli lint --help + +# Modules that 'mxcli lint' skips entirely. +# +# System is Mendix's own platform module — its entities, members and access +# rules are not yours to change, so findings against it are noise. On a blank +# app it accounts for the large majority of all issues. +# +# Marketplace modules (Atlas_Core, Atlas_Web_Content, Administration, …) are +# equally read-only in practice; add them here if their findings distract you. +# +# NOTE: this list always wins. It is merged with '--exclude', and a module +# listed here is skipped even if you ask for it with '--modules'. To lint +# System, remove it from this list (or delete this file). +excludeModules: + - System + +# Per-rule overrides. Examples: +# +# rules: +# QUAL002: # missing documentation +# enabled: false +# CONV009: # max microflow objects +# severity: warning +# options: +# maxObjects: 20 +rules: {} +` + +// writeDefaultLintConfig creates .claude/lint-config.yaml unless the project +// already has a lint config in any of the locations linter.FindConfigFile +// searches. Never overwrites: init is re-runnable, and the config is meant to +// be edited. +func writeDefaultLintConfig(projectDir string) (string, bool) { + if existing := linter.FindConfigFile(projectDir); existing != "" { + return existing, false + } + claudeDir := filepath.Join(projectDir, ".claude") + if err := os.MkdirAll(claudeDir, 0755); err != nil { + fmt.Fprintf(os.Stderr, " Error creating .claude directory for lint config: %v\n", err) + return "", false + } + path := filepath.Join(claudeDir, "lint-config.yaml") + if err := os.WriteFile(path, []byte(defaultLintConfig), 0644); err != nil { + fmt.Fprintf(os.Stderr, " Error writing lint config: %v\n", err) + return "", false + } + return path, true +} + func findMprFile(dir string) string { entries, err := os.ReadDir(dir) if err != nil { diff --git a/cmd/mxcli/init_lint_config_test.go b/cmd/mxcli/init_lint_config_test.go new file mode 100644 index 000000000..60705bdcd --- /dev/null +++ b/cmd/mxcli/init_lint_config_test.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// `mxcli init` seeds a lint config excluding System. On a blank app the System +// module accounted for ~96% of all lint findings — entities you cannot +// document, give access rules to, or rename — which buried the findings about +// the developer's own code (issuetracker finding #9). +func TestWriteDefaultLintConfig(t *testing.T) { + t.Run("creates the config and excludes System", func(t *testing.T) { + dir := t.TempDir() + + path, created := writeDefaultLintConfig(dir) + if !created { + t.Fatal("expected the config to be created in an empty project") + } + if want := filepath.Join(dir, ".claude", "lint-config.yaml"); path != want { + t.Errorf("path = %q, want %q", path, want) + } + + // It must parse as a real config, not just look right. + cfg, err := linter.LoadConfig(path) + if err != nil { + t.Fatalf("the seeded config does not load: %v", err) + } + if len(cfg.ExcludeModules) != 1 || cfg.ExcludeModules[0] != "System" { + t.Errorf("ExcludeModules = %v, want [System]", cfg.ExcludeModules) + } + + // And lint must actually find it where it looks. + if found := linter.FindConfigFile(dir); found != path { + t.Errorf("FindConfigFile = %q, want %q — lint would not pick it up", found, path) + } + }) + + t.Run("never overwrites an existing config", func(t *testing.T) { + // init is re-runnable and the config is meant to be edited, so a second + // run must not discard the developer's changes. + for _, existing := range []string{ + filepath.Join(".claude", "lint-config.yaml"), + "lint-config.yaml", + ".lint-config.yaml", + } { + t.Run(existing, func(t *testing.T) { + dir := t.TempDir() + full := filepath.Join(dir, existing) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatal(err) + } + const mine = "excludeModules: [MyOwnModule]\n" + if err := os.WriteFile(full, []byte(mine), 0644); err != nil { + t.Fatal(err) + } + + if _, created := writeDefaultLintConfig(dir); created { + t.Error("reported creating a config when one already existed") + } + got, err := os.ReadFile(full) + if err != nil { + t.Fatal(err) + } + if string(got) != mine { + t.Errorf("existing config was modified:\n%s", got) + } + // It must not have written a competing config elsewhere either. + other := filepath.Join(dir, ".claude", "lint-config.yaml") + if other != full { + if _, err := os.Stat(other); err == nil { + t.Error("wrote a second, competing config at .claude/lint-config.yaml") + } + } + }) + } + }) +} + +// An exclude beats --modules (LintContext.IsExcluded checks the exclude set +// first), so once init ships a config excluding System, `lint -m System` +// returns nothing. intersect drives the warning that explains why. +func TestIntersect(t *testing.T) { + tests := []struct { + name string + want []string + have []string + out []string + }{ + {name: "shadowed module detected", want: []string{"System"}, have: []string{"System"}, out: []string{"System"}}, + {name: "unshadowed module ignored", want: []string{"MyModule"}, have: []string{"System"}, out: nil}, + { + name: "only the overlap, in the caller's order", + want: []string{"MyModule", "System", "Atlas_Core"}, + have: []string{"Atlas_Core", "System"}, + out: []string{"System", "Atlas_Core"}, + }, + {name: "duplicates collapse", want: []string{"System", "System"}, have: []string{"System"}, out: []string{"System"}}, + {name: "no filter", want: nil, have: []string{"System"}, out: nil}, + {name: "no excludes", want: []string{"System"}, have: nil, out: nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := intersect(tc.want, tc.have) + if strings.Join(got, ",") != strings.Join(tc.out, ",") { + t.Errorf("intersect(%v, %v) = %v, want %v", tc.want, tc.have, got, tc.out) + } + }) + } +} diff --git a/docs-site/src/ide/init-output.md b/docs-site/src/ide/init-output.md index 513bbde24..d26baaa54 100644 --- a/docs-site/src/ide/init-output.md +++ b/docs-site/src/ide/init-output.md @@ -42,10 +42,29 @@ your-mendix-project/ ├── settings.json # Claude Code project settings ├── commands/ # Slash commands for Claude │ └── mendix/ # Mendix-specific commands -└── lint-rules/ # Starlark lint rules +├── lint-rules/ # Starlark lint rules +└── lint-config.yaml # Lint settings (excluded modules, rule overrides) CLAUDE.md # Project context for Claude ``` +#### `lint-config.yaml` + +Seeded with the **System module excluded**. System is Mendix's own platform +module — you cannot document its entities, give them access rules, or rename +their members — so linting it produces findings you can never action. On a +blank app that was the large majority of all issues. + +`mxcli init` writes this file only when the project has no lint config yet +(`.claude/lint-config.yaml`, `lint-config.yaml`, or `.lint-config.yaml`), so +re-running init never discards your edits. + +Add Marketplace modules (`Atlas_Core`, `Administration`, …) to `excludeModules` +if their findings distract you — they are equally read-only in practice. + +To lint System after all, remove it from `excludeModules`. Note the list always +wins: it merges with `--exclude`, and a module listed there stays excluded even +if you name it with `--modules` (lint warns when you try). + ### Cursor ``` From 4949de96b510f6c391b48c3120dacf4e43a7c946 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 21:54:31 +0000 Subject: [PATCH 2/5] fix(oql): take the column set from all rows, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Mendix runtime omits a column from a row's JSON object when its value is null. parseOQLFeedback took the column list from the first row only, so a column that happened to be null in row 1 was dropped from the entire result — `mxcli oql` rendered a narrower table than the query asked for, with no error and no empty column to hint at the loss. The column set is now the union of every row's keys. New keys are inserted directly after the last key already known rather than appended, so a column absent from earlier rows keeps its SELECT position: merging [A, C] with [A, B, C] gives [A, B, C], not [A, C, B]. Rows are re-scanned for key order only when they carry a key not seen yet, so the uniform case still costs one length check. Verified by stubbing the union back to first-row-only and watching the reported symptom return. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- cmd/mxcli/docker/oql.go | 78 +++++++++++++++++++++++++++++++---- cmd/mxcli/docker/oql_test.go | 79 ++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 8 deletions(-) diff --git a/cmd/mxcli/docker/oql.go b/cmd/mxcli/docker/oql.go index 6e5d71e0d..b41b10395 100644 --- a/cmd/mxcli/docker/oql.go +++ b/cmd/mxcli/docker/oql.go @@ -169,20 +169,40 @@ func parseOQLFeedback(rawFeedback json.RawMessage) (*OQLResult, error) { return result, nil } - // Extract column order from the first row using json.Decoder Token() method - columns, err := extractColumnOrder(rows[0]) - if err != nil { - return nil, fmt.Errorf("extracting columns: %w", err) - } - result.Columns = columns - - // Parse each row preserving column order + // The column set is the union of the keys of every row, not the keys of the + // first one: the runtime omits a column from a row's JSON object when its + // value is null, so a column that happens to be null in row 1 is absent + // there and would otherwise be dropped from the whole result — silently + // answering a different query than the one that was asked. + var columns []string + known := make(map[string]bool) + rowMaps := make([]map[string]any, 0, len(rows)) for _, rawRow := range rows { var rowMap map[string]any if err := json.Unmarshal(rawRow, &rowMap); err != nil { return nil, fmt.Errorf("parsing row: %w", err) } + rowMaps = append(rowMaps, rowMap) + + // Re-scanning a row for key order is only needed when it carries a + // column not seen yet; the common case (every row has the same keys) + // costs one length check. + if !hasOnlyKnownKeys(rowMap, known) { + keys, err := extractColumnOrder(rawRow) + if err != nil { + return nil, fmt.Errorf("extracting columns: %w", err) + } + columns = mergeColumnOrder(columns, keys) + for _, col := range columns { + known[col] = true + } + } + } + result.Columns = columns + // Project each row onto the merged column order. A column missing from a + // row is a null value, which formats as NULL. + for _, rowMap := range rowMaps { row := make([]any, len(columns)) for i, col := range columns { row[i] = rowMap[col] @@ -193,6 +213,48 @@ func parseOQLFeedback(rawFeedback json.RawMessage) (*OQLResult, error) { return result, nil } +// hasOnlyKnownKeys reports whether every key of rowMap is already a known column. +func hasOnlyKnownKeys(rowMap map[string]any, known map[string]bool) bool { + if len(rowMap) > len(known) { + return false + } + for key := range rowMap { + if !known[key] { + return false + } + } + return true +} + +// mergeColumnOrder folds one row's key order into the accumulated column list. +// +// New keys are inserted directly after the last key that was already known, +// rather than appended, so a column absent from earlier rows still lands in its +// SELECT position: merging [A, C] with [A, B, C] yields [A, B, C], not +// [A, C, B]. +func mergeColumnOrder(columns []string, rowKeys []string) []string { + index := make(map[string]int, len(columns)) + for i, col := range columns { + index[col] = i + } + + insertAt := 0 // just past the last key of this row found in columns + for _, key := range rowKeys { + if pos, ok := index[key]; ok { + insertAt = pos + 1 + continue + } + columns = append(columns, "") + copy(columns[insertAt+1:], columns[insertAt:]) + columns[insertAt] = key + for i := insertAt; i < len(columns); i++ { + index[columns[i]] = i + } + insertAt++ + } + return columns +} + // extractColumnOrder uses json.Decoder to preserve key order from a JSON object. func extractColumnOrder(raw json.RawMessage) ([]string, error) { dec := json.NewDecoder(bytes.NewReader(raw)) diff --git a/cmd/mxcli/docker/oql_test.go b/cmd/mxcli/docker/oql_test.go index 84aa1ef89..7d12bb90e 100644 --- a/cmd/mxcli/docker/oql_test.go +++ b/cmd/mxcli/docker/oql_test.go @@ -316,6 +316,85 @@ func TestExecuteOQL_ColumnOrder(t *testing.T) { } } +// TestParseOQLFeedback_ColumnUnionAcrossRows covers the runtime's habit of +// omitting a column from a row's JSON object when its value is null. Taking the +// column set from row 1 alone dropped such a column from the entire result, so +// the table silently answered a narrower query than the one that was asked. +func TestParseOQLFeedback_ColumnUnionAcrossRows(t *testing.T) { + tests := []struct { + name string + feedback string + want []string + wantRows [][]any + }{ + { + name: "column null in first row survives", + feedback: `{"data":[{"Name":"Alice"},{"Name":"Bob","Nickname":"Bobby"}]}`, + want: []string{"Name", "Nickname"}, + wantRows: [][]any{{"Alice", nil}, {"Bob", "Bobby"}}, + }, + { + name: "missing middle column keeps its SELECT position", + feedback: `{"data":[{"A":"a1","C":"c1"},{"A":"a2","B":"b2","C":"c2"}]}`, + want: []string{"A", "B", "C"}, + wantRows: [][]any{{"a1", nil, "c1"}, {"a2", "b2", "c2"}}, + }, + { + name: "column null in every row but one is still reported", + feedback: `{"data":[{"A":"a1"},{"A":"a2"},{"A":"a3","B":"b3"}]}`, + want: []string{"A", "B"}, + wantRows: [][]any{{"a1", nil}, {"a2", nil}, {"a3", "b3"}}, + }, + { + name: "uniform rows keep first-row order", + feedback: `{"data":[{"Zebra":"z","Alpha":"a"},{"Zebra":"z2","Alpha":"a2"}]}`, + want: []string{"Zebra", "Alpha"}, + wantRows: [][]any{{"z", "a"}, {"z2", "a2"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseOQLFeedback(json.RawMessage(tt.feedback)) + if err != nil { + t.Fatalf("parseOQLFeedback: %v", err) + } + if got := fmt.Sprintf("%v", result.Columns); got != fmt.Sprintf("%v", tt.want) { + t.Errorf("columns: got %v, want %v", result.Columns, tt.want) + } + if got, want := fmt.Sprintf("%v", result.Rows), fmt.Sprintf("%v", tt.wantRows); got != want { + t.Errorf("rows: got %s, want %s", got, want) + } + }) + } +} + +func TestMergeColumnOrder(t *testing.T) { + tests := []struct { + name string + columns []string + keys []string + want string + }{ + {"first row seeds the order", nil, []string{"A", "B"}, "[A B]"}, + {"known keys change nothing", []string{"A", "B"}, []string{"A", "B"}, "[A B]"}, + {"new key inserted in position", []string{"A", "C"}, []string{"A", "B", "C"}, "[A B C]"}, + {"new leading key goes first", []string{"B"}, []string{"A", "B"}, "[A B]"}, + {"new trailing key goes last", []string{"A"}, []string{"A", "B"}, "[A B]"}, + {"two new keys keep their order", []string{"A", "D"}, []string{"A", "B", "C", "D"}, "[A B C D]"}, + {"row with only unknown keys appends", []string{"A"}, []string{"B", "C"}, "[B C A]"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := fmt.Sprintf("%v", mergeColumnOrder(tt.columns, tt.keys)) + if got != tt.want { + t.Errorf("mergeColumnOrder(%v, %v) = %s, want %s", tt.columns, tt.keys, got, tt.want) + } + }) + } +} + // parseTestServerAddr extracts host and port from an httptest server URL. func parseTestServerAddr(t *testing.T, rawURL string) (string, int) { t.Helper() From e28c0b41cbcd57a0164d5609173fa0fb13f0751f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 21:54:46 +0000 Subject: [PATCH 3/5] feat(check): reject a loop variable used outside its loop (MDL053) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Referencing a loop's variable after `end loop;` passed `mxcli check` and then failed the build with [error] [CE0108] "Variable 'item' is defined but not in scope at this location." Both flavours reproduce against mxbuild 11.12.1: the iterator itself, and anything the loop body introduces (a retrieve, a `$X = create …`, a call output). MDL053 maps each loop-scoped name to the loop whose own body introduces it (a nested loop keeps its own names), then walks the flow tracking which loops enclose the current position and flags any reference from outside the owner. A name claimed by two loops is deliberately not reported — that is the MDL052/CE0111 duplicate-name case, and without the guard the existing MDL052 negative example started failing for the wrong reason. MDL052 is the sibling rule: names are unique across the whole microflow, but visibility stops at the loop body. The write-microflows skill now states both and shows the carry-out idiom. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 2 + .claude/skills/mendix/write-microflows.md | 27 ++ ...oku-40-loop-variable-out-of-scope.fail.mdl | 56 +++ mdl/executor/validate_microflow.go | 5 + mdl/executor/validate_microflow_loop_scope.go | 356 ++++++++++++++++++ .../validate_microflow_loop_scope_test.go | 209 ++++++++++ 6 files changed, 655 insertions(+) create mode 100644 mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl create mode 100644 mdl/executor/validate_microflow_loop_scope.go create mode 100644 mdl/executor/validate_microflow_loop_scope_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index e0775b351..320dac719 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -356,6 +356,8 @@ cases for these three BSON types — they fell to `default: return nil`. | An unquoted negative number in an XPath constraint fails to parse: `where [Amount > -7]` → `Parse error: extraneous input '7' expecting {',', ')'}`. Reported as "negative numeric literals truncate (`-7` becomes `-`)" | `xpathWord` — the name-part rule inside XPath — is a **negated token set** that did not exclude `MINUS`, so the sign was consumed as a name word and the digits were left stranded (hence the truncation appearance). The lexer deliberately keeps `-` out of `NUMBER_LITERAL` (a leading sign there mis-tokenises `$x -2`), leaving negation to the parser; the general grammar has `unaryExpression` for this and the XPath grammar simply never got the equivalent | `mdl/grammar/domains/MDLPage.g4` (`xpathValueExpr` gains `MINUS xpathValueExpr`; `MINUS` added to the `xpathWord` exclusion set), `mdl/visitor/visitor_xpath.go` (`buildXPathValueExpr`), `mdl/visitor/visitor_page_v3.go` (`xpathExprToString` emits `-7`, not `- 7`) | **The grammar fix alone is worse than the bug.** With the parser accepting `-7` but the XPath AST builder having no case for the new alternative, the constraint parses and silently serializes to `[Amount > ]` — a dropped operand instead of a loud parse error. Caught only because the visitor has a round-trip helper; the microflow write path uses `GetText()` and looked fine. **When adding a grammar alternative, check every consumer of that rule, not just the one your repro exercises.** **Scope correction**: the finding's own example (`addDays([%CurrentDateTime%], -7)`) still fails — `addDays` is a *microflow expression* function, not an XPath one, and it fails `CE0161` with a POSITIVE argument too, so the sign was never its problem. Repro `mdl-examples/bug-tests/it-18-xpath-negative-literal.mdl`; A/B on Mendix 11.12.1: pre-fix the script does not parse, fixed binary writes it and `mx check` reports 0 errors. issuetracker #18 | | Text painted by an **Atlas topbar widget is invisible in a dark theme** — the language selector measures ~1.13:1 contrast, glyph pixels spanning 4 luminance values out of 255. A theme override exists and *names the right element*, so it looks handled | Two separate mistakes stacked. (1) **Specificity**: Atlas's own rule is `.navbar-brand .widget-language-selector .current-language-text` at (0,3,0); a bare `.current-language-text` at (0,1,0) never wins, and only appears to on layouts that do not nest the selector under `.navbar-brand`. (2) **Wrong value**: `color: inherit` inherits *body ink*, which is dark, while the rail is dark in both palettes — so even at the winning specificity it measures 1.00:1. Atlas paints from `--bg-color-secondary` with a `#fff` fallback because it assumes a dark rail | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-atlas-map.scss` (the "Atlas fixups" block) | Re-declare **Atlas's own selector shape** so the weights match and source order decides, and resolve the colour through the rail token (`var(--mxt-rail-ink-active, var(--mxt-rail-ink))`) rather than `inherit`. List the bare and the `.navbar-brand`-nested selectors together — each is matched at its own specificity, so one rule covers both layouts. **Generalisable — the shape to look for**: a guard that names the right element is not evidence it applies. Read the *winning* declaration (`CSS.getMatchedStylesForNode` in DevTools, or the computed value) instead of the one you wrote. **Measure contrast, not colour**: reading `getComputedStyle(el).color` once and seeing a plausible value proves nothing — compute the WCAG ratio against the first non-transparent ancestor background, which is what turns "looks fine" into 1.13 vs 19.47. Reported from the RssReader test build; tests in `cmd/mxcli/theme/theme_test.go`, verified in a browser at 17.79:1 light / 19.47:1 dark | +| A loop's variable used after `end loop;` passes `mxcli check`, then `mx check` fails `[error] [CE0108] "Variable 'item' is defined but not in scope at this location."` at the referencing activity. Applies to the **iterator** and to anything the body introduces (a `retrieve`, a `$X = create …`, a call output) | Nothing tracked loop-variable *visibility*. MDL052 already covered the sibling rule — names are unique across the whole microflow (CE0111) — and the wording of that rule ("scoped to the WHOLE microflow") reads as if the variable is readable flow-wide. Uniqueness and visibility are different: the name is reserved everywhere, readable only inside the loop body | `mdl/executor/validate_microflow_loop_scope.go` (new `MDL053`, wired from `microflowValidator.validate`), skill `.claude/skills/mendix/write-microflows.md` | Map each loop-scoped name to the loop whose **own** body introduces it (nested loops keep their own names), then walk the flow with the set of enclosing loops and flag any reference from outside the owner. **A name claimed by two loops is dropped, not reported** — that is the MDL052/CE0111 case, and without the guard the MDL052 negative example started failing for the wrong reason: the first loop's own use of `$R` was blamed on the second loop's claim. **Generalisable**: a rule keyed by variable *name* needs an ambiguity escape hatch whenever another rule exists precisely because names can collide. Both flavours verified against mxbuild 11.12.1 (2 × CE0108 in one probe). Repro `mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl`; tests `mdl/executor/validate_microflow_loop_scope_test.go`. sudoku #40 | +| `mxcli oql` silently omits a whole column: a `select A, B …` renders only `A`, and the JSON output has no `B` key at all — no error, no empty column. Reproduces whenever the value of `B` is **null in the first row** | The runtime omits a null-valued column from a row's JSON object entirely, and `parseOQLFeedback` took the column list from `extractColumnOrder(rows[0])` — one row, chosen for its key *order*. Every later row was then projected onto that short list, so the column vanished from the result rather than showing NULLs | `cmd/mxcli/docker/oql.go` (`parseOQLFeedback`, `mergeColumnOrder`, `hasOnlyKnownKeys`) | Union the keys of **all** rows, inserting each new key directly after the last key already known rather than appending — merging `[A, C]` with `[A, B, C]` must give `[A, B, C]`, not `[A, C, B]`, or a column that is null early in the result set jumps to the end of the table. The per-row re-scan is skipped when a row carries no unseen key, so the uniform case still costs one length check. **Generalisable**: any "take the shape from the first element" over a sparse encoding is a silent-wrong-answer bug, not a formatting bug — the output looks complete. Tests `TestParseOQLFeedback_ColumnUnionAcrossRows`, `TestMergeColumnOrder`; proven by stubbing the union back to first-row-only and watching the reported symptom return. sudoku #39 | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index 524532123..efdc57f08 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -491,6 +491,33 @@ end loop; - The loop variable type is **automatically derived** from the list type (e.g., `list of Test.Product` → `Test.Product`) - CHANGE statements inside loops use the derived type to resolve attribute names +> **Nothing a loop defines survives past `end loop;`.** The iterator *and* +> anything the body creates (a `retrieve`, a `$X = create …`, a call output) are +> visible only inside the body; using one afterwards is +> `CE0108 "Variable 'X' is defined but not in scope at this location."` +> (`mxcli check` flags it as **MDL053**). +> +> ```mdl +> -- WRONG: $Last is created inside the loop, read outside it +> loop $Item in $Items +> begin +> $Last = create Test.Product (Name = $Item/Name); +> end loop; +> commit $Last; -- MDL053 / CE0108 +> +> -- RIGHT: declare before the loop, assign inside, read after +> declare $LastName string = ''; +> loop $Item in $Items +> begin +> set $LastName = $Item/Name; +> end loop; +> log info node 'Test' $LastName; +> ``` +> +> Visibility and *naming* are separate rules: names must also be unique across +> the **whole** microflow, so two loops cannot share an iterator name either +> (`CE0111`, flagged as **MDL052**). + ### Performance: Batch Commit After Loop **CRITICAL**: Do NOT commit inside a loop. Each `commit` inside a loop issues a separate database transaction, which causes N round-trips for N records and degrades performance significantly. diff --git a/mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl b/mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl new file mode 100644 index 000000000..560124bad --- /dev/null +++ b/mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl @@ -0,0 +1,56 @@ +-- ============================================================================ +-- Sudoku finding #40: a loop variable used after the loop passed mxcli check +-- ============================================================================ +-- +-- Symptom (before fix): referencing a loop's variable AFTER `end loop;` passed +-- `mxcli check` cleanly, and `mx check` then failed with +-- +-- [error] [CE0108] "Variable 'item' is defined but not in scope at this +-- location." at Change object activity +-- +-- Two flavours, both reproduced against mxbuild 11.12.1: +-- 1. the loop ITERATOR itself ($item below) +-- 2. anything the loop BODY introduces ($Inner below) — a retrieve, a create, +-- a call output +-- +-- Note the sibling rule: MDL052 rejects two loops REUSING an iterator name, +-- because Mendix requires variable names to be unique across the whole +-- microflow (CE0111). Uniqueness and visibility are different things — the name +-- is reserved flow-wide, but it is only readable inside the loop body. +-- +-- After fix: MDL053 rejects both flavours at check time and points at the +-- carry-out idiom (declare before the loop, assign inside, read after). +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl +-- ============================================================================ + +create module S40; + +create entity S40.Thing ( + Label : string(200) +); + +-- (1) The loop iterator, used after the loop → MDL053 / CE0108. +create microflow S40.MF_IteratorAfterLoop ( + $Things: list of S40.Thing +) +begin + loop $item in $Things + begin + change $item (Label = 'in loop'); + end loop; + change $item (Label = 'after loop'); +end; + +-- (2) A variable created inside the loop body, used after the loop → same. +create microflow S40.MF_BodyVarAfterLoop ( + $Things: list of S40.Thing +) +begin + loop $t in $Things + begin + $Inner = create S40.Thing (Label = 'x'); + end loop; + change $Inner (Label = 'after loop'); +end; diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index a765a106e..c05dee69a 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -88,6 +88,11 @@ func (v *microflowValidator) validate(body []ast.MicroflowStatement) { // Duplicate loop iterator names — a Mendix loop variable is scoped to the whole // microflow, so reusing a name across loops is CE0111 at build time. v.checkDuplicateLoopVariables(body) + + // The other half of that rule: names are unique flow-wide, but a loop's + // variables are only VISIBLE inside its body, so using one after the loop + // is CE0108. + v.checkLoopScoping(body) } // checkDuplicateLoopVariables flags a loop iterator name used by more than one diff --git a/mdl/executor/validate_microflow_loop_scope.go b/mdl/executor/validate_microflow_loop_scope.go new file mode 100644 index 000000000..ef95ee682 --- /dev/null +++ b/mdl/executor/validate_microflow_loop_scope.go @@ -0,0 +1,356 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// checkLoopScoping flags a reference, from outside a loop, to a variable that +// only exists inside it — the loop iterator itself, or anything the loop body +// introduces (a retrieve, a create, a call output, …). +// +// Mendix scopes both to the loop body, so the reference builds as +// +// [CE0108] "Variable 'X' is defined but not in scope at this location." +// +// even though the flow is otherwise well-formed. MDL052 is the sibling rule for +// the other half of Mendix's loop-variable semantics: names must be unique +// across the WHOLE microflow (CE0111), while visibility stops at the loop body. +// +// Only names owned by a loop are considered, and any name that is also +// introduced outside a loop is dropped from the set first — so a re-declared +// name can never be reported. +func (v *microflowValidator) checkLoopScoping(body []ast.MicroflowStatement) { + owner := map[string]*ast.LoopStmt{} + ambiguous := map[string]bool{} + collectLoopScopedVars(body, owner, ambiguous) + if len(owner) == 0 { + return + } + + // A name claimed by two loops has no single owning body to judge references + // against — and it is already MDL052/CE0111 ("Duplicate variable name"). + for name := range ambiguous { + delete(owner, name) + } + + // A name introduced outside any loop is in scope after the loop regardless + // of what the loop does with it; never report those. + for name := range collectNonLoopDeclaredVars(body) { + delete(owner, name) + } + if len(owner) == 0 { + return + } + + v.walkLoopScope(body, map[*ast.LoopStmt]bool{}, owner, map[string]bool{}) +} + +// walkLoopScope walks the body with the set of loops whose bodies enclose the +// current position, reporting each out-of-scope name once. +func (v *microflowValidator) walkLoopScope( + body []ast.MicroflowStatement, + active map[*ast.LoopStmt]bool, + owner map[string]*ast.LoopStmt, + reported map[string]bool, +) { + for _, s := range body { + for _, name := range loopRefVars(s) { + if name == "" || reported[name] { + continue + } + loop, ok := owner[name] + if !ok || active[loop] { + continue + } + reported[name] = true + v.addViolation("MDL053", linter.SeverityError, + fmt.Sprintf("variable '$%s' is used outside the loop over '$%s' that defines it; "+ + "a Mendix loop variable — the iterator and anything the loop body creates — "+ + "is scoped to the loop body, so this builds as CE0108 "+ + "\"Variable '%s' is defined but not in scope at this location\"", + name, loop.ListVariable, name), + fmt.Sprintf("Move the statement into the loop body, or carry the value out "+ + "in a variable declared before the loop (e.g. 'declare $Result …' then "+ + "'set $Result = $%s' inside the loop)", name)) + } + + // Recurse into nested bodies, extending the active set for loops. + switch st := s.(type) { + case *ast.LoopStmt: + inner := make(map[*ast.LoopStmt]bool, len(active)+1) + for l := range active { + inner[l] = true + } + inner[st] = true + v.walkLoopScope(st.Body, inner, owner, reported) + continue + case *ast.WhileStmt: + v.walkLoopScope(st.Body, active, owner, reported) + case *ast.IfStmt: + v.walkLoopScope(st.ThenBody, active, owner, reported) + v.walkLoopScope(st.ElseBody, active, owner, reported) + case *ast.EnumSplitStmt: + for _, c := range st.Cases { + v.walkLoopScope(c.Body, active, owner, reported) + } + v.walkLoopScope(st.ElseBody, active, owner, reported) + case *ast.InheritanceSplitStmt: + for _, c := range st.Cases { + v.walkLoopScope(c.Body, active, owner, reported) + } + v.walkLoopScope(st.ElseBody, active, owner, reported) + } + + if eh := stmtErrorHandling(s); eh != nil && len(eh.Body) > 0 { + v.walkLoopScope(eh.Body, active, owner, reported) + } + } +} + +// collectLoopScopedVars maps every loop-scoped variable name to the loop whose +// body introduces it. Each loop claims only the names in its OWN body — a +// nested loop's names belong to the nested loop — so a name that still ends up +// claimed twice is a genuine duplicate (two sibling loops reusing an iterator); +// those go into ambiguous and are not reported here. +func collectLoopScopedVars(body []ast.MicroflowStatement, owner map[string]*ast.LoopStmt, ambiguous map[string]bool) { + claim := func(name string, loop *ast.LoopStmt) { + if name == "" { + return + } + if prev, seen := owner[name]; seen && prev != loop { + ambiguous[name] = true + } + owner[name] = loop + } + + for _, s := range body { + switch st := s.(type) { + case *ast.LoopStmt: + claim(st.LoopVariable, st) + for name := range declaredVarsOwnScope(st.Body) { + claim(name, st) + } + collectLoopScopedVars(st.Body, owner, ambiguous) + case *ast.WhileStmt: + collectLoopScopedVars(st.Body, owner, ambiguous) + case *ast.IfStmt: + collectLoopScopedVars(st.ThenBody, owner, ambiguous) + collectLoopScopedVars(st.ElseBody, owner, ambiguous) + case *ast.EnumSplitStmt: + for _, c := range st.Cases { + collectLoopScopedVars(c.Body, owner, ambiguous) + } + collectLoopScopedVars(st.ElseBody, owner, ambiguous) + case *ast.InheritanceSplitStmt: + for _, c := range st.Cases { + collectLoopScopedVars(c.Body, owner, ambiguous) + } + collectLoopScopedVars(st.ElseBody, owner, ambiguous) + } + if eh := stmtErrorHandling(s); eh != nil && len(eh.Body) > 0 { + collectLoopScopedVars(eh.Body, owner, ambiguous) + } + } +} + +// collectNonLoopDeclaredVars returns the names introduced anywhere OUTSIDE a +// loop body (branches and error handlers included — those are a different +// scoping question, covered by MDL005). +func collectNonLoopDeclaredVars(body []ast.MicroflowStatement) map[string]bool { + vars := map[string]bool{} + var walk func([]ast.MicroflowStatement) + walk = func(stmts []ast.MicroflowStatement) { + for _, s := range stmts { + for name := range collectDeclaredVars([]ast.MicroflowStatement{s}) { + vars[name] = true + } + switch st := s.(type) { + case *ast.LoopStmt: + // Deliberately not descended into: those names are loop-scoped. + case *ast.WhileStmt: + walk(st.Body) + case *ast.IfStmt: + walk(st.ThenBody) + walk(st.ElseBody) + case *ast.EnumSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + case *ast.InheritanceSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + } + if eh := stmtErrorHandling(s); eh != nil && len(eh.Body) > 0 { + walk(eh.Body) + } + } + } + walk(body) + return vars +} + +// declaredVarsOwnScope returns the variable names a loop body introduces +// itself — descending into branches, whiles, and error handlers, but NOT into a +// nested loop, whose names belong to that loop. +func declaredVarsOwnScope(body []ast.MicroflowStatement) map[string]bool { + vars := map[string]bool{} + var walk func([]ast.MicroflowStatement) + walk = func(stmts []ast.MicroflowStatement) { + for _, s := range stmts { + for name := range collectDeclaredVars([]ast.MicroflowStatement{s}) { + vars[name] = true + } + switch st := s.(type) { + case *ast.LoopStmt: + // Owned by the nested loop, not by this body. + case *ast.WhileStmt: + walk(st.Body) + case *ast.IfStmt: + walk(st.ThenBody) + walk(st.ElseBody) + case *ast.EnumSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + case *ast.InheritanceSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + } + if eh := stmtErrorHandling(s); eh != nil && len(eh.Body) > 0 { + walk(eh.Body) + } + } + } + walk(body) + return vars +} + +// loopRefVars returns the variable names a single statement reads, WITHOUT +// descending into nested bodies (walkLoopScope visits those itself, so that a +// reference is judged against the loops actually enclosing it). +// +// A statement kind missing here only costs a missed report, never a false one. +func loopRefVars(stmt ast.MicroflowStatement) []string { + var refs []string + add := func(names ...string) { + for _, n := range names { + if n != "" { + refs = append(refs, extractVarName(n)) + } + } + } + addExpr := func(exprs ...ast.Expression) { + for _, e := range exprs { + refs = append(refs, exprVarRefs(e)...) + } + } + addArgs := func(args []ast.CallArgument) { + for _, a := range args { + addExpr(a.Value) + } + } + addChanges := func(items []ast.ChangeItem) { + for _, c := range items { + addExpr(c.Value) + } + } + + switch s := stmt.(type) { + case *ast.MfSetStmt: + add(s.Target) + addExpr(s.Value) + case *ast.DeclareStmt: + addExpr(s.InitialValue) + case *ast.ReturnStmt: + addExpr(s.Value) + case *ast.CreateObjectStmt: + addChanges(s.Changes) + case *ast.ChangeObjectStmt: + add(s.Variable) + addChanges(s.Changes) + case *ast.MfCommitStmt: + add(s.Variable) + case *ast.DeleteObjectStmt: + add(s.Variable) + case *ast.RollbackStmt: + add(s.Variable) + case *ast.RetrieveStmt: + add(s.StartVariable) + addExpr(s.Where) + case *ast.IfStmt: + addExpr(s.Condition) + case *ast.WhileStmt: + addExpr(s.Condition) + case *ast.LoopStmt: + // The iterator is defined here, not referenced; the list is not. + add(s.ListVariable) + case *ast.EnumSplitStmt: + add(s.Variable) + case *ast.InheritanceSplitStmt: + add(s.Variable) + case *ast.CastObjectStmt: + add(s.ObjectVariable) + case *ast.LogStmt: + addExpr(s.Node, s.Message) + case *ast.CallMicroflowStmt: + addArgs(s.Arguments) + case *ast.CallNanoflowStmt: + addArgs(s.Arguments) + case *ast.CallJavaActionStmt: + addArgs(s.Arguments) + case *ast.CallJavaScriptActionStmt: + addArgs(s.Arguments) + case *ast.ExecuteDatabaseQueryStmt: + addArgs(s.Arguments) + addArgs(s.ConnectionArguments) + case *ast.ListOperationStmt: + add(s.InputVariable, s.SecondVariable) + addExpr(s.Condition, s.OffsetExpr, s.LimitExpr) + case *ast.AggregateListStmt: + add(s.InputVariable) + addExpr(s.Expression) + case *ast.AddToListStmt: + add(s.List) + if s.Value != nil { + addExpr(s.Value) + } else { + add(s.Item) + } + case *ast.RemoveFromListStmt: + add(s.Item, s.List) + case *ast.ShowPageStmt: + add(s.ForObject) + for _, a := range s.Arguments { + addExpr(a.Value) + } + case *ast.ShowMessageStmt: + addExpr(s.Message) + addExpr(s.TemplateArgs...) + case *ast.DownloadFileStmt: + add(s.FileDocument) + case *ast.ValidationFeedbackStmt: + if s.AttributePath != nil { + add(s.AttributePath.Variable) + } + addExpr(s.Message) + addExpr(s.TemplateArgs...) + } + + // Normalise: strip any leftover $ sigils from expression-derived names. + for i, r := range refs { + refs[i] = strings.TrimPrefix(r, "$") + } + return refs +} diff --git a/mdl/executor/validate_microflow_loop_scope_test.go b/mdl/executor/validate_microflow_loop_scope_test.go new file mode 100644 index 000000000..87201adac --- /dev/null +++ b/mdl/executor/validate_microflow_loop_scope_test.go @@ -0,0 +1,209 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// loopScopeViolations parses an MDL source containing exactly one microflow and +// returns the MDL053 messages the validator produces for it. +func loopScopeViolations(t *testing.T, src string) []string { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var msgs []string + for _, stmt := range prog.Statements { + mf, ok := stmt.(*ast.CreateMicroflowStmt) + if !ok { + continue + } + for _, v := range ValidateMicroflow(mf) { + if v.RuleID == "MDL053" { + msgs = append(msgs, v.Message) + } + } + } + return msgs +} + +// TestLoopScope_IteratorUsedAfterLoop is the reported symptom: the loop +// iterator is referenced after `end loop`. Verified against mxbuild 11.12.1 as +// [CE0108] "Variable 'item' is defined but not in scope at this location." +func TestLoopScope_IteratorUsedAfterLoop(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Items: list of Sample.Thing) +begin + loop $item in $Items + begin + change $item (Name = 'in'); + end loop; + change $item (Name = 'after'); +end; +`) + if len(msgs) != 1 { + t.Fatalf("expected 1 MDL053, got %d: %v", len(msgs), msgs) + } + if !strings.Contains(msgs[0], "$item") || !strings.Contains(msgs[0], "CE0108") { + t.Errorf("message should name the variable and CE0108, got: %s", msgs[0]) + } +} + +// A variable CREATED inside the loop body is loop-scoped too — same CE0108, +// confirmed against mxbuild 11.12.1. +func TestLoopScope_BodyVariableUsedAfterLoop(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Items: list of Sample.Thing) +begin + loop $item in $Items + begin + $Inner = create Sample.Thing (Name = 'x'); + end loop; + change $Inner (Name = 'after'); +end; +`) + if len(msgs) != 1 { + t.Fatalf("expected 1 MDL053, got %d: %v", len(msgs), msgs) + } + if !strings.Contains(msgs[0], "$Inner") { + t.Errorf("message should name $Inner, got: %s", msgs[0]) + } +} + +// An inner loop's variable used in the outer loop body — after the inner +// `end loop` but still inside the outer one — is equally out of scope. +func TestLoopScope_InnerLoopVariableUsedInOuterBody(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Outer: list of Sample.Thing, $Inner: list of Sample.Thing) +begin + loop $o in $Outer + begin + loop $i in $Inner + begin + change $i (Name = 'in'); + end loop; + change $i (Name = 'outer body'); + end loop; +end; +`) + if len(msgs) != 1 { + t.Fatalf("expected 1 MDL053, got %d: %v", len(msgs), msgs) + } + if !strings.Contains(msgs[0], "$i") { + t.Errorf("message should name $i, got: %s", msgs[0]) + } +} + +// A reference from a branch that follows the loop is just as out of scope as a +// reference at the top level. +func TestLoopScope_ReferenceInBranchAfterLoop(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Items: list of Sample.Thing) +begin + loop $item in $Items + begin + log info node 'Sample' 'x'; + end loop; + if $Items != empty then + change $item (Name = 'after'); + end if; +end; +`) + if len(msgs) != 1 { + t.Fatalf("expected 1 MDL053, got %d: %v", len(msgs), msgs) + } +} + +// Everything that stays inside the loop body — including a nested branch and a +// nested loop reading the outer iterator — must not be reported. +func TestLoopScope_UsesInsideLoopAreClean(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Items: list of Sample.Thing, $Others: list of Sample.Thing) +begin + declare $Total integer = 0; + loop $item in $Items + begin + if $item/Name != empty then + set $Total = $Total + 1; + end if; + loop $other in $Others + begin + change $other (Name = $item/Name); + end loop; + change $item (Name = 'still in scope'); + end loop; + return $Total; +end; +`) + if len(msgs) != 0 { + t.Fatalf("expected no MDL053, got: %v", msgs) + } +} + +// The carry-out idiom — declare before the loop, assign inside, read after — +// is the recommended fix and must stay clean. +func TestLoopScope_CarryOutVariableIsClean(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($Items: list of Sample.Thing) +begin + declare $LastName string = ''; + loop $item in $Items + begin + set $LastName = $item/Name; + end loop; + log info node 'Sample' $LastName; +end; +`) + if len(msgs) != 0 { + t.Fatalf("expected no MDL053, got: %v", msgs) + } +} + +// Two sibling loops REUSING an iterator name have no single owning body, so +// MDL053 must stay silent and leave the case to MDL052 (CE0111). Without this +// the first loop's own use of the name was reported as out of scope. +func TestLoopScope_DuplicateIteratorNameNotReported(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($A: list of Sample.Thing, $B: list of Sample.Thing) +begin + loop $R in $A + begin + change $R (Name = 'a'); + end loop; + loop $R in $B + begin + change $R (Name = 'b'); + end loop; +end; +`) + if len(msgs) != 0 { + t.Fatalf("expected no MDL053 for a duplicate iterator name (MDL052 owns it), got: %v", msgs) + } +} + +// Two sequential loops each using their own iterator: no cross-references, so +// nothing to report. +func TestLoopScope_SequentialLoopsAreClean(t *testing.T) { + msgs := loopScopeViolations(t, ` +create microflow Sample.MF ($A: list of Sample.Thing, $B: list of Sample.Thing) +begin + loop $a in $A + begin + change $a (Name = 'a'); + end loop; + loop $b in $B + begin + change $b (Name = 'b'); + end loop; +end; +`) + if len(msgs) != 0 { + t.Fatalf("expected no MDL053, got: %v", msgs) + } +} From b472da26d4c4f5129d02a947afd8036a9337ee95 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 22:08:23 +0000 Subject: [PATCH 4/5] docs: correct the attribution of the loop-scope and OQL fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop-variable-scope check (MDL053) was filed against sudoku finding #40, which is actually an app bug in that project ("roughly one dealt board in twenty is not solvable by forced logic") and explicitly marked "not an mxcli bug". The check is a real, mxbuild-verified check-parity gap, so it stays — but it is not that finding, and the repro file and symptom row said otherwise. The OQL column fix does match sudoku #39, but only its first half; the second (ORDER BY on a DateTime attribute being ignored) is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 4 ++-- ...-of-scope.fail.mdl => loop-variable-out-of-scope.fail.mdl} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename mdl-examples/bug-tests/{sudoku-40-loop-variable-out-of-scope.fail.mdl => loop-variable-out-of-scope.fail.mdl} (91%) diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 320dac719..4b9e7afc4 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -356,8 +356,8 @@ cases for these three BSON types — they fell to `default: return nil`. | An unquoted negative number in an XPath constraint fails to parse: `where [Amount > -7]` → `Parse error: extraneous input '7' expecting {',', ')'}`. Reported as "negative numeric literals truncate (`-7` becomes `-`)" | `xpathWord` — the name-part rule inside XPath — is a **negated token set** that did not exclude `MINUS`, so the sign was consumed as a name word and the digits were left stranded (hence the truncation appearance). The lexer deliberately keeps `-` out of `NUMBER_LITERAL` (a leading sign there mis-tokenises `$x -2`), leaving negation to the parser; the general grammar has `unaryExpression` for this and the XPath grammar simply never got the equivalent | `mdl/grammar/domains/MDLPage.g4` (`xpathValueExpr` gains `MINUS xpathValueExpr`; `MINUS` added to the `xpathWord` exclusion set), `mdl/visitor/visitor_xpath.go` (`buildXPathValueExpr`), `mdl/visitor/visitor_page_v3.go` (`xpathExprToString` emits `-7`, not `- 7`) | **The grammar fix alone is worse than the bug.** With the parser accepting `-7` but the XPath AST builder having no case for the new alternative, the constraint parses and silently serializes to `[Amount > ]` — a dropped operand instead of a loud parse error. Caught only because the visitor has a round-trip helper; the microflow write path uses `GetText()` and looked fine. **When adding a grammar alternative, check every consumer of that rule, not just the one your repro exercises.** **Scope correction**: the finding's own example (`addDays([%CurrentDateTime%], -7)`) still fails — `addDays` is a *microflow expression* function, not an XPath one, and it fails `CE0161` with a POSITIVE argument too, so the sign was never its problem. Repro `mdl-examples/bug-tests/it-18-xpath-negative-literal.mdl`; A/B on Mendix 11.12.1: pre-fix the script does not parse, fixed binary writes it and `mx check` reports 0 errors. issuetracker #18 | | Text painted by an **Atlas topbar widget is invisible in a dark theme** — the language selector measures ~1.13:1 contrast, glyph pixels spanning 4 luminance values out of 255. A theme override exists and *names the right element*, so it looks handled | Two separate mistakes stacked. (1) **Specificity**: Atlas's own rule is `.navbar-brand .widget-language-selector .current-language-text` at (0,3,0); a bare `.current-language-text` at (0,1,0) never wins, and only appears to on layouts that do not nest the selector under `.navbar-brand`. (2) **Wrong value**: `color: inherit` inherits *body ink*, which is dark, while the rail is dark in both palettes — so even at the winning specificity it measures 1.00:1. Atlas paints from `--bg-color-secondary` with a `#fff` fallback because it assumes a dark rail | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-atlas-map.scss` (the "Atlas fixups" block) | Re-declare **Atlas's own selector shape** so the weights match and source order decides, and resolve the colour through the rail token (`var(--mxt-rail-ink-active, var(--mxt-rail-ink))`) rather than `inherit`. List the bare and the `.navbar-brand`-nested selectors together — each is matched at its own specificity, so one rule covers both layouts. **Generalisable — the shape to look for**: a guard that names the right element is not evidence it applies. Read the *winning* declaration (`CSS.getMatchedStylesForNode` in DevTools, or the computed value) instead of the one you wrote. **Measure contrast, not colour**: reading `getComputedStyle(el).color` once and seeing a plausible value proves nothing — compute the WCAG ratio against the first non-transparent ancestor background, which is what turns "looks fine" into 1.13 vs 19.47. Reported from the RssReader test build; tests in `cmd/mxcli/theme/theme_test.go`, verified in a browser at 17.79:1 light / 19.47:1 dark | -| A loop's variable used after `end loop;` passes `mxcli check`, then `mx check` fails `[error] [CE0108] "Variable 'item' is defined but not in scope at this location."` at the referencing activity. Applies to the **iterator** and to anything the body introduces (a `retrieve`, a `$X = create …`, a call output) | Nothing tracked loop-variable *visibility*. MDL052 already covered the sibling rule — names are unique across the whole microflow (CE0111) — and the wording of that rule ("scoped to the WHOLE microflow") reads as if the variable is readable flow-wide. Uniqueness and visibility are different: the name is reserved everywhere, readable only inside the loop body | `mdl/executor/validate_microflow_loop_scope.go` (new `MDL053`, wired from `microflowValidator.validate`), skill `.claude/skills/mendix/write-microflows.md` | Map each loop-scoped name to the loop whose **own** body introduces it (nested loops keep their own names), then walk the flow with the set of enclosing loops and flag any reference from outside the owner. **A name claimed by two loops is dropped, not reported** — that is the MDL052/CE0111 case, and without the guard the MDL052 negative example started failing for the wrong reason: the first loop's own use of `$R` was blamed on the second loop's claim. **Generalisable**: a rule keyed by variable *name* needs an ambiguity escape hatch whenever another rule exists precisely because names can collide. Both flavours verified against mxbuild 11.12.1 (2 × CE0108 in one probe). Repro `mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl`; tests `mdl/executor/validate_microflow_loop_scope_test.go`. sudoku #40 | -| `mxcli oql` silently omits a whole column: a `select A, B …` renders only `A`, and the JSON output has no `B` key at all — no error, no empty column. Reproduces whenever the value of `B` is **null in the first row** | The runtime omits a null-valued column from a row's JSON object entirely, and `parseOQLFeedback` took the column list from `extractColumnOrder(rows[0])` — one row, chosen for its key *order*. Every later row was then projected onto that short list, so the column vanished from the result rather than showing NULLs | `cmd/mxcli/docker/oql.go` (`parseOQLFeedback`, `mergeColumnOrder`, `hasOnlyKnownKeys`) | Union the keys of **all** rows, inserting each new key directly after the last key already known rather than appending — merging `[A, C]` with `[A, B, C]` must give `[A, B, C]`, not `[A, C, B]`, or a column that is null early in the result set jumps to the end of the table. The per-row re-scan is skipped when a row carries no unseen key, so the uniform case still costs one length check. **Generalisable**: any "take the shape from the first element" over a sparse encoding is a silent-wrong-answer bug, not a formatting bug — the output looks complete. Tests `TestParseOQLFeedback_ColumnUnionAcrossRows`, `TestMergeColumnOrder`; proven by stubbing the union back to first-row-only and watching the reported symptom return. sudoku #39 | +| A loop's variable used after `end loop;` passes `mxcli check`, then `mx check` fails `[error] [CE0108] "Variable 'item' is defined but not in scope at this location."` at the referencing activity. Applies to the **iterator** and to anything the body introduces (a `retrieve`, a `$X = create …`, a call output) | Nothing tracked loop-variable *visibility*. MDL052 already covered the sibling rule — names are unique across the whole microflow (CE0111) — and the wording of that rule ("scoped to the WHOLE microflow") reads as if the variable is readable flow-wide. Uniqueness and visibility are different: the name is reserved everywhere, readable only inside the loop body | `mdl/executor/validate_microflow_loop_scope.go` (new `MDL053`, wired from `microflowValidator.validate`), skill `.claude/skills/mendix/write-microflows.md` | Map each loop-scoped name to the loop whose **own** body introduces it (nested loops keep their own names), then walk the flow with the set of enclosing loops and flag any reference from outside the owner. **A name claimed by two loops is dropped, not reported** — that is the MDL052/CE0111 case, and without the guard the MDL052 negative example started failing for the wrong reason: the first loop's own use of `$R` was blamed on the second loop's claim. **Generalisable**: a rule keyed by variable *name* needs an ambiguity escape hatch whenever another rule exists precisely because names can collide. Both flavours verified against mxbuild 11.12.1 (2 × CE0108 in one probe). Repro `mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl`; tests `mdl/executor/validate_microflow_loop_scope_test.go`. Found while working the sudoku findings, but **not** one of them — the numbered finding it was filed under is an app bug in that project, not an mxcli defect | +| `mxcli oql` silently omits a whole column: a `select A, B …` renders only `A`, and the JSON output has no `B` key at all — no error, no empty column. Reproduces whenever the value of `B` is **null in the first row** | The runtime omits a null-valued column from a row's JSON object entirely, and `parseOQLFeedback` took the column list from `extractColumnOrder(rows[0])` — one row, chosen for its key *order*. Every later row was then projected onto that short list, so the column vanished from the result rather than showing NULLs | `cmd/mxcli/docker/oql.go` (`parseOQLFeedback`, `mergeColumnOrder`, `hasOnlyKnownKeys`) | Union the keys of **all** rows, inserting each new key directly after the last key already known rather than appending — merging `[A, C]` with `[A, B, C]` must give `[A, B, C]`, not `[A, C, B]`, or a column that is null early in the result set jumps to the end of the table. The per-row re-scan is skipped when a row carries no unseen key, so the uniform case still costs one length check. **Generalisable**: any "take the shape from the first element" over a sparse encoding is a silent-wrong-answer bug, not a formatting bug — the output looks complete. Tests `TestParseOQLFeedback_ColumnUnionAcrossRows`, `TestMergeColumnOrder`; proven by stubbing the union back to first-row-only and watching the reported symptom return. sudoku #39, first half | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl b/mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl similarity index 91% rename from mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl rename to mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl index 560124bad..10110ad63 100644 --- a/mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl +++ b/mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl @@ -1,5 +1,5 @@ -- ============================================================================ --- Sudoku finding #40: a loop variable used after the loop passed mxcli check +-- Loop variable used after the loop passed mxcli check (CE0108) -- ============================================================================ -- -- Symptom (before fix): referencing a loop's variable AFTER `end loop;` passed @@ -22,7 +22,7 @@ -- carry-out idiom (declare before the loop, assign inside, read after). -- -- Usage (expected to FAIL check): --- mxcli check mdl-examples/bug-tests/sudoku-40-loop-variable-out-of-scope.fail.mdl +-- mxcli check mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl -- ============================================================================ create module S40; From c9639021da570faf85f3a56f2615cca2794124b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 22:21:28 +0000 Subject: [PATCH 5/5] feat(test): run microflow tests without Docker (mxcli test --local) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mxcli test` got all the way through parsing, runner generation, model injection, after-startup wiring and the full mxbuild build with no container, then died on `docker up` — so microflow tests were unavailable in exactly the environments mxcli targets, where /usr/bin/docker exists with no daemon behind it. Only the last step needed a container. --local boots the same standalone runtime `mxcli run --local` uses (new docker.StartLocalApp: cache mxbuild + runtime, ensure the database, mxbuild --serve build, boot) and reads the runner's output from the runtime log. The Docker path moves to runDockerAndCapture; both share parse, inject, parse-results and cleanup. Local runs use their own ports (8081/8091) and a _test database, so a warm `run --local` loop can keep serving the same project and test data never lands in the database the developer is looking at. Two things only running it could have taught: - The runner reports through an after-startup microflow, so its LOG output happens during the start action, before the runtime attaches its log subscriber. Registering the subscriber early is not possible — the runtime answers LoggingException pre-start. The JVM console tee, live from spawn, is what carries it; confirmed by A/B running with the early attach removed, and it is documented where it matters. - A failing test makes the runner return false, which fails the after-startup action, which makes `start` return an error. The first version reported that as a broken run and printed a stack trace instead of the test report. Verified end-to-end in a container with no Docker daemon: two tests passing, then one passing and one failing with exit 1, project restored both times. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .claude/skills/mendix/test-microflows.md | 20 +- CLAUDE.md | 2 +- cmd/mxcli/cmd_test_run.go | 12 +- cmd/mxcli/docker/localapp.go | 223 ++++++++++++++++++++++ cmd/mxcli/docker/runlocal.go | 6 + cmd/mxcli/main.go | 1 + cmd/mxcli/testrunner/runner.go | 71 ++++--- cmd/mxcli/testrunner/runner_local.go | 174 +++++++++++++++++ cmd/mxcli/testrunner/runner_local_test.go | 135 +++++++++++++ docs-site/src/migration/validation.md | 2 +- 11 files changed, 614 insertions(+), 33 deletions(-) create mode 100644 cmd/mxcli/docker/localapp.go create mode 100644 cmd/mxcli/testrunner/runner_local.go create mode 100644 cmd/mxcli/testrunner/runner_local_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 4b9e7afc4..8011b92d8 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -358,6 +358,7 @@ cases for these three BSON types — they fell to `default: return nil`. | Text painted by an **Atlas topbar widget is invisible in a dark theme** — the language selector measures ~1.13:1 contrast, glyph pixels spanning 4 luminance values out of 255. A theme override exists and *names the right element*, so it looks handled | Two separate mistakes stacked. (1) **Specificity**: Atlas's own rule is `.navbar-brand .widget-language-selector .current-language-text` at (0,3,0); a bare `.current-language-text` at (0,1,0) never wins, and only appears to on layouts that do not nest the selector under `.navbar-brand`. (2) **Wrong value**: `color: inherit` inherits *body ink*, which is dark, while the rail is dark in both palettes — so even at the winning specificity it measures 1.00:1. Atlas paints from `--bg-color-secondary` with a `#fff` fallback because it assumes a dark rail | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-atlas-map.scss` (the "Atlas fixups" block) | Re-declare **Atlas's own selector shape** so the weights match and source order decides, and resolve the colour through the rail token (`var(--mxt-rail-ink-active, var(--mxt-rail-ink))`) rather than `inherit`. List the bare and the `.navbar-brand`-nested selectors together — each is matched at its own specificity, so one rule covers both layouts. **Generalisable — the shape to look for**: a guard that names the right element is not evidence it applies. Read the *winning* declaration (`CSS.getMatchedStylesForNode` in DevTools, or the computed value) instead of the one you wrote. **Measure contrast, not colour**: reading `getComputedStyle(el).color` once and seeing a plausible value proves nothing — compute the WCAG ratio against the first non-transparent ancestor background, which is what turns "looks fine" into 1.13 vs 19.47. Reported from the RssReader test build; tests in `cmd/mxcli/theme/theme_test.go`, verified in a browser at 17.79:1 light / 19.47:1 dark | | A loop's variable used after `end loop;` passes `mxcli check`, then `mx check` fails `[error] [CE0108] "Variable 'item' is defined but not in scope at this location."` at the referencing activity. Applies to the **iterator** and to anything the body introduces (a `retrieve`, a `$X = create …`, a call output) | Nothing tracked loop-variable *visibility*. MDL052 already covered the sibling rule — names are unique across the whole microflow (CE0111) — and the wording of that rule ("scoped to the WHOLE microflow") reads as if the variable is readable flow-wide. Uniqueness and visibility are different: the name is reserved everywhere, readable only inside the loop body | `mdl/executor/validate_microflow_loop_scope.go` (new `MDL053`, wired from `microflowValidator.validate`), skill `.claude/skills/mendix/write-microflows.md` | Map each loop-scoped name to the loop whose **own** body introduces it (nested loops keep their own names), then walk the flow with the set of enclosing loops and flag any reference from outside the owner. **A name claimed by two loops is dropped, not reported** — that is the MDL052/CE0111 case, and without the guard the MDL052 negative example started failing for the wrong reason: the first loop's own use of `$R` was blamed on the second loop's claim. **Generalisable**: a rule keyed by variable *name* needs an ambiguity escape hatch whenever another rule exists precisely because names can collide. Both flavours verified against mxbuild 11.12.1 (2 × CE0108 in one probe). Repro `mdl-examples/bug-tests/loop-variable-out-of-scope.fail.mdl`; tests `mdl/executor/validate_microflow_loop_scope_test.go`. Found while working the sudoku findings, but **not** one of them — the numbered finding it was filed under is an app bug in that project, not an mxcli defect | | `mxcli oql` silently omits a whole column: a `select A, B …` renders only `A`, and the JSON output has no `B` key at all — no error, no empty column. Reproduces whenever the value of `B` is **null in the first row** | The runtime omits a null-valued column from a row's JSON object entirely, and `parseOQLFeedback` took the column list from `extractColumnOrder(rows[0])` — one row, chosen for its key *order*. Every later row was then projected onto that short list, so the column vanished from the result rather than showing NULLs | `cmd/mxcli/docker/oql.go` (`parseOQLFeedback`, `mergeColumnOrder`, `hasOnlyKnownKeys`) | Union the keys of **all** rows, inserting each new key directly after the last key already known rather than appending — merging `[A, C]` with `[A, B, C]` must give `[A, B, C]`, not `[A, C, B]`, or a column that is null early in the result set jumps to the end of the table. The per-row re-scan is skipped when a row carries no unseen key, so the uniform case still costs one length check. **Generalisable**: any "take the shape from the first element" over a sparse encoding is a silent-wrong-answer bug, not a formatting bug — the output looks complete. Tests `TestParseOQLFeedback_ColumnUnionAcrossRows`, `TestMergeColumnOrder`; proven by stubbing the union back to first-row-only and watching the reported symptom return. sudoku #39, first half | +| `mxcli test` cannot run at all in a container without a Docker daemon — parsing, runner generation, model injection and the **entire mxbuild build** succeed natively, then `docker up` fails with "failed to connect to the docker API at unix:///var/run/docker.sock". So microflow tests are unavailable in exactly the environment mxcli targets (Claude Code web containers ship `/usr/bin/docker` with no daemon) | Only one step of the run was containerised — start the runtime and read its log — but it was wired directly to `docker compose`, with no seam for another way to run the app. `run --local` had all three pieces already (boot a standalone runtime, tee its log, restore) | `cmd/mxcli/docker/localapp.go` (new `StartLocalApp`), `cmd/mxcli/testrunner/runner_local.go` (new), `cmd/mxcli/testrunner/runner.go` (docker path extracted to `runDockerAndCapture`), `cmd/mxcli/cmd_test_run.go` + `main.go` (`--local`) | Give the run a seam — `runLocalAndCapture` / `runDockerAndCapture` — so both modes share parse/inject/parse-results/cleanup and differ only in how the app is started. **Two traps, both found by running it rather than reasoning about it.** (a) The runner reports via an **after-startup** microflow, so its LOG output happens DURING the start action, before the runtime's log subscriber attaches; registering the subscriber early is not possible (the runtime answers `LoggingException` pre-start). What actually carries the output is the JVM console tee, live from spawn — verified by A/B running with the early attach removed. (b) A **failing** test makes the runner return false, which makes the after-startup action fail, which makes `start` return an error — the first version reported that as a broken run and dumped a stack trace instead of the test report. A failed boot whose log shows a verdict is a normal outcome. Local runs use their own ports (8081/8091) and a `_test` database so a `run --local` dev loop can keep serving. Verified end-to-end in a daemon-less container: 2 passed; then 1 passed / 1 failed with exit 1, project restored. sudoku #41 | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/.claude/skills/mendix/test-microflows.md b/.claude/skills/mendix/test-microflows.md index 90420179a..3a2ce469a 100644 --- a/.claude/skills/mendix/test-microflows.md +++ b/.claude/skills/mendix/test-microflows.md @@ -16,8 +16,22 @@ For **UI/page testing** (widget rendering, form interactions, browser tests), se ## Prerequisites - Mendix project with microflows to test -- Docker stack initialized: `mxcli docker init -p app.mpr` -- App buildable: `mxcli docker build -p app.mpr` +- A way to run the app — **either** of: + - `--local` (no Docker): mxcli boots the runtime itself, the same way + `mxcli run --local` does. This is the only option in a container without a + Docker daemon, which includes Claude Code web sessions. + - Docker: stack initialized (`mxcli docker init -p app.mpr`) and the app + buildable (`mxcli docker build -p app.mpr`). + +```bash +mxcli test tests/ -p app.mpr --local # no daemon needed +mxcli test tests/ -p app.mpr # Docker +``` + +`--local` uses its own ports (app 8081, admin 8091) and its own +`_test` database, so a `mxcli run --local` dev loop can keep serving the +same project while the tests run — the tests never write into the database you +are looking at in the browser. The database is created on first use. --- @@ -114,7 +128,7 @@ The test runner uses the **after-startup microflow** pattern: module already exists 3. Generates a `MxTest.TestRunner` microflow with assertion logic and points after-startup at it -4. Builds the project and restarts the Docker runtime +4. Builds the project and restarts the runtime (Docker, or local with `--local`) 5. Captures structured `MXTEST:` log lines for pass/fail 6. Restores the original after-startup setting and removes the generated runner — the whole `MxTest` module when the runner created it, otherwise just the diff --git a/CLAUDE.md b/CLAUDE.md index 071b69322..571858a38 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -496,7 +496,7 @@ go build -o bin/mxcli ./cmd/mxcli | **Full-text search** | `search 'keyword'` | Search across all strings and source | | **Linting** | `mxcli lint -p app.mpr [--format json\|sarif]` | 15 built-in rules + 27 Starlark rules (MDL, SEC, QUAL, ARCH, DESIGN, CONV) | | **Report** | `mxcli report -p app.mpr [--format markdown\|json\|html]` | Scored best practices report with category breakdown | -| **Testing** | `mxcli test tests/ -p app.mpr` | `.test.mdl` / `.test.md` files, requires Docker | +| **Testing** | `mxcli test tests/ -p app.mpr [--local]` | `.test.mdl` / `.test.md` files; `--local` runs on mxcli's own runtime (no Docker daemon), on its own ports + `_test` database | | **Diff** | `mxcli diff -p app.mpr changes.mdl` | Compare script against project state | | **Diff local** | `mxcli diff-local -p app.mpr --ref head` | Git diff for MPR v2 projects | | **Diff revisions** | `mxcli diff-local -p app.mpr --ref main..feature` | Compare two arbitrary git revisions | diff --git a/cmd/mxcli/cmd_test_run.go b/cmd/mxcli/cmd_test_run.go index ac30574b2..74254dccf 100644 --- a/cmd/mxcli/cmd_test_run.go +++ b/cmd/mxcli/cmd_test_run.go @@ -31,10 +31,15 @@ The test runner: 1. Parses test files and extracts test blocks with @test/@expect annotations 2. Generates a TestRunner microflow 3. Injects it into the project as after-startup microflow -4. Builds and restarts the Mendix runtime in Docker +4. Builds and restarts the Mendix runtime (Docker, or --local) 5. Captures structured log output to determine pass/fail 6. Restores original project settings +With --local the app runs on mxcli's own runtime instead of a container — the +same boot as 'mxcli run --local', so no Docker daemon is needed. It uses its own +ports (8081/8091) and its own '_test' database, so a warm 'run --local' +loop can keep serving the same project while tests run. + Supports two file formats: .test.mdl — Pure MDL test blocks separated by / .test.md — Markdown specification with embedded mdl-test code blocks @@ -52,6 +57,9 @@ Examples: # List tests without executing mxcli test tests/ -p app.mpr --list + # Run without Docker, on mxcli's own local runtime + mxcli test tests/ -p app.mpr --local + # Skip build (reuse existing deployment) mxcli test tests/ -p app.mpr --skip-build @@ -64,6 +72,7 @@ Examples: list, _ := cmd.Flags().GetBool("list") junitOutput, _ := cmd.Flags().GetString("junit") skipBuild, _ := cmd.Flags().GetBool("skip-build") + local, _ := cmd.Flags().GetBool("local") verbose, _ := cmd.Flags().GetBool("verbose") color, _ := cmd.Flags().GetBool("color") timeoutStr, _ := cmd.Flags().GetString("timeout") @@ -93,6 +102,7 @@ Examples: ProjectPath: projectPath, TestFiles: args, SkipBuild: skipBuild, + Local: local, Timeout: timeout, JUnitOutput: junitOutput, Verbose: verbose, diff --git a/cmd/mxcli/docker/localapp.go b/cmd/mxcli/docker/localapp.go new file mode 100644 index 000000000..cae6a7429 --- /dev/null +++ b/cmd/mxcli/docker/localapp.go @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "fmt" + "io" + "os" + "path/filepath" + "time" + + "github.com/mendixlabs/mxcli/sdk/mpr" +) + +// LocalAppOptions configures StartLocalApp. +type LocalAppOptions struct { + // ProjectPath is the .mpr file. + ProjectPath string + // DeployDir is the deployment directory (default /deployment). + DeployDir string + // AppPort / AdminPort / ServePort default to 8080 / 8090 / 6543. + AppPort int + AdminPort int + ServePort int + // AdminPass is the M2EE admin password (defaults to the local-run password). + AdminPass string + // DB is the database to connect to; empty fields take the run --local + // defaults (PostgreSQL at 127.0.0.1:5432, user/password mendix, database + // name derived from the project file name). + DB DBConfig + // EnsureDB provisions the local Postgres + database when missing instead of + // only checking reachability. + EnsureDB bool + // SkipBuild boots against whatever is already in DeployDir. + SkipBuild bool + // RuntimeLogPath tees the runtime JVM output and the runtime's own + // application log to this file. + RuntimeLogPath string + // Stdout/Stderr receive progress messages. + Stdout io.Writer + Stderr io.Writer +} + +// LocalApp is a booted local app: an mxbuild serve server plus the standalone +// runtime it deployed to. It is the Docker-free equivalent of `docker compose +// up` for callers that need an app running and then stopped again. +type LocalApp struct { + Runtime *LocalRuntime + // Version is the project's Mendix version. + Version string + // RuntimeLogPath is where the runtime log is being written (may be empty). + RuntimeLogPath string + + serve *ServeServer +} + +func (o *LocalAppOptions) applyDefaults() { + if o.DeployDir == "" { + o.DeployDir = filepath.Join(filepath.Dir(o.ProjectPath), "deployment") + } + if o.AppPort == 0 { + o.AppPort = 8080 + } + if o.AdminPort == 0 { + o.AdminPort = 8090 + } + if o.ServePort == 0 { + o.ServePort = 6543 + } + if o.AdminPass == "" { + o.AdminPass = defaultLocalAdminPass + } + if o.DB.Type == "" { + o.DB.Type = "PostgreSQL" + } + if o.DB.Host == "" { + o.DB.Host = "127.0.0.1:5432" + } + if o.DB.User == "" { + o.DB.User = "mendix" + } + if o.DB.Password == "" { + o.DB.Password = "mendix" + } + if o.DB.Name == "" { + o.DB.Name = deriveDBName(o.ProjectPath) + } + if o.Stdout == nil { + o.Stdout = os.Stdout + } + if o.Stderr == nil { + o.Stderr = os.Stderr + } +} + +// StartLocalApp builds the project with mxbuild and boots the standalone +// runtime against the result — the same sequence as `mxcli run --local`, minus +// everything a headless caller does not need (no web client bundle, no hub, no +// watch loop, no screenshots). +// +// The caller owns the returned app and must Stop it. +func StartLocalApp(opts LocalAppOptions) (*LocalApp, error) { + opts.applyDefaults() + w := opts.Stdout + + if err := checkLocalAppPortsFree(opts); err != nil { + return nil, err + } + + // 1. Project version → which mxbuild and runtime to use. + reader, err := mpr.Open(opts.ProjectPath) + if err != nil { + return nil, fmt.Errorf("opening project: %w", err) + } + version := reader.ProjectVersion().ProductVersion + reader.Close() + + // 2. Cache mxbuild + runtime (no-ops when already present). + if _, err := DownloadMxBuild(version, w); err != nil { + return nil, fmt.Errorf("setting up mxbuild: %w", err) + } + installPath, err := resolveRuntimeInstall(version, w) + if err != nil { + return nil, fmt.Errorf("setting up runtime: %w", err) + } + + // 3. Database. + if opts.EnsureDB { + if err := EnsureDatabase(opts.DB, w); err != nil { + return nil, fmt.Errorf("ensuring database: %w", err) + } + } else if err := pingTCP(opts.DB.Host, 3*time.Second); err != nil { + return nil, fmt.Errorf("database not reachable at %s: %w\n"+ + " Pass --ensure-db to provision it, or start Postgres and create the %q database (user %q).", + opts.DB.Host, err, opts.DB.Name, opts.DB.User) + } + + app := &LocalApp{Version: version, RuntimeLogPath: opts.RuntimeLogPath} + + // 4. Build, unless the caller is reusing an existing deployment. + if !opts.SkipBuild { + fmt.Fprintln(w, "Building project (mxbuild --serve)...") + serve, err := StartServe(ServeOptions{Version: version, Host: "127.0.0.1", Port: opts.ServePort}) + if err != nil { + return nil, fmt.Errorf("starting mxbuild serve: %w", err) + } + app.serve = serve + + build, err := serve.Build(BuildRequest{Target: TargetDeploy, ProjectFilePath: opts.ProjectPath}) + if err != nil { + app.Stop() + return nil, fmt.Errorf("build: %w", err) + } + if !build.OK() { + app.Stop() + return nil, fmt.Errorf("build failed: %s\n%s", build.Message, string(build.Raw)) + } + } + + // 5. Boot the runtime against the deployment. + rt, err := StartLocalRuntime(LocalRuntimeOptions{ + DeployDir: opts.DeployDir, + InstallPath: installPath, + AppPort: opts.AppPort, + AdminPort: opts.AdminPort, + AdminPass: opts.AdminPass, + DB: opts.DB, + RuntimeLogPath: opts.RuntimeLogPath, + Stdout: opts.Stdout, + Stderr: opts.Stderr, + }) + if err != nil { + app.Stop() + return nil, err + } + app.Runtime = rt + return app, nil +} + +// Stop shuts down the runtime and the build server. Safe to call more than once +// and on a partially-started app. +func (a *LocalApp) Stop() error { + var firstErr error + if a.Runtime != nil { + if err := a.Runtime.Stop(); err != nil { + firstErr = err + } + a.Runtime = nil + } + if a.serve != nil { + if err := a.serve.Stop(); err != nil && firstErr == nil { + firstErr = err + } + a.serve = nil + } + return firstErr +} + +// checkLocalAppPortsFree refuses to boot onto a port something is already +// serving — otherwise a stale runtime is silently adopted and the caller reads +// results from an app it did not build. +func checkLocalAppPortsFree(o LocalAppOptions) error { + ports := []struct { + port int + what string + }{ + {o.AppPort, "app"}, + {o.AdminPort, "runtime admin API"}, + } + if !o.SkipBuild { + ports = append(ports, struct { + port int + what string + }{o.ServePort, "mxbuild serve"}) + } + for _, p := range ports { + if err := pingTCP(fmt.Sprintf("127.0.0.1:%d", p.port), 300*time.Millisecond); err == nil { + return fmt.Errorf("port %d (%s) is already in use — stop the running instance first, "+ + "or pass a different port", p.port, p.what) + } + } + return nil +} diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index 1353a1658..8cc54c8a0 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -235,6 +235,12 @@ func parseRuntimeSetting(s string) (string, any, error) { // deriveDBName turns a project file name into a safe Postgres database name: // lowercased, non-alphanumerics collapsed to underscores, leading digit prefixed. +// DeriveDBName is the local-run database name for a project: the .mpr file name +// lowercased and sanitised to a legal identifier. Exported so callers that boot +// their own local app (e.g. the test runner, which appends a suffix to keep test +// data out of the dev database) derive the same base name. +func DeriveDBName(projectPath string) string { return deriveDBName(projectPath) } + func deriveDBName(projectPath string) string { base := strings.TrimSuffix(filepath.Base(projectPath), filepath.Ext(projectPath)) var b strings.Builder diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index 27cbf146e..3703823a3 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -370,6 +370,7 @@ func init() { testRunCmd.Flags().BoolP("list", "l", false, "List tests without executing") testRunCmd.Flags().StringP("junit", "j", "", "Write JUnit XML results to file") testRunCmd.Flags().BoolP("skip-build", "s", false, "Skip build step (reuse existing deployment)") + testRunCmd.Flags().Bool("local", false, "Run on mxcli's local runtime instead of Docker (no daemon needed)") testRunCmd.Flags().BoolP("verbose", "v", false, "Show all runtime log output") testRunCmd.Flags().BoolP("color", "", false, "Use colored output") testRunCmd.Flags().StringP("timeout", "t", "5m", "Timeout for runtime startup and test execution") diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index f0d35c25e..b82fa06f9 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -37,6 +37,10 @@ type RunOptions struct { // SkipBuild skips the MxBuild step (reuse existing deployment). SkipBuild bool + // Local runs the app with mxcli's own local runtime (`run --local`) instead + // of a Docker container. Everything else about the run is unchanged. + Local bool + // Timeout for runtime startup and test execution. Timeout time.Duration @@ -138,36 +142,18 @@ func Run(opts RunOptions) (*SuiteResult, error) { } fmt.Fprintf(w, " After-startup set to %s\n", mxTestRunner) - // Step 4: Build and restart - dockerDir := filepath.Join(filepath.Dir(opts.ProjectPath), ".docker") - if err := ensureDockerStack(opts.ProjectPath, dockerDir, w); err != nil { - reportCleanup(w, cleanup(opts.ProjectPath, state, w)) - return nil, fmt.Errorf("docker init: %w", err) - } - - if !opts.SkipBuild { - fmt.Fprintln(w, "Building project...") - if err := execMxcli(opts.ProjectPath, "docker", "build", "-p", opts.ProjectPath, "--skip-check"); err != nil { - reportCleanup(w, cleanup(opts.ProjectPath, state, w)) - return nil, fmt.Errorf("docker build: %w", err) - } - } - - fmt.Fprintln(w, "Restarting runtime...") - // Stop existing containers - runCompose(dockerDir, "down") - // Start fresh - if err := runCompose(dockerDir, "up", "--detach", "--force-recreate"); err != nil { - reportCleanup(w, cleanup(opts.ProjectPath, state, w)) - return nil, fmt.Errorf("docker up: %w", err) + // Steps 4+5: build, run the app, and capture the runner's log output. The + // local and Docker paths differ only in how the app is started and where its + // log is read from; everything before and after is shared. + var logOutput string + if opts.Local { + logOutput, err = runLocalAndCapture(opts, timeout, w) + } else { + logOutput, err = runDockerAndCapture(opts, timeout, w) } - - // Step 5: Wait for runtime and capture logs - fmt.Fprintf(w, "Waiting for test execution (timeout: %s)...\n", timeout) - logOutput, err := captureRuntimeLogs(dockerDir, timeout, w, opts.Verbose) if err != nil { reportCleanup(w, cleanup(opts.ProjectPath, state, w)) - return nil, fmt.Errorf("runtime execution: %w", err) + return nil, err } // Step 6: Parse results from logs @@ -454,6 +440,37 @@ func reportCleanup(w io.Writer, err error) { fmt.Fprintf(w, "Check the after-startup microflow and the %s module before committing.\n", mxTestModule) } +// runDockerAndCapture builds the project, restarts the compose stack, and reads +// the test runner's output from the container log. +func runDockerAndCapture(opts RunOptions, timeout time.Duration, w io.Writer) (string, error) { + dockerDir := filepath.Join(filepath.Dir(opts.ProjectPath), ".docker") + if err := ensureDockerStack(opts.ProjectPath, dockerDir, w); err != nil { + return "", fmt.Errorf("docker init: %w", err) + } + + if !opts.SkipBuild { + fmt.Fprintln(w, "Building project...") + if err := execMxcli(opts.ProjectPath, "docker", "build", "-p", opts.ProjectPath, "--skip-check"); err != nil { + return "", fmt.Errorf("docker build: %w", err) + } + } + + fmt.Fprintln(w, "Restarting runtime...") + runCompose(dockerDir, "down") + if err := runCompose(dockerDir, "up", "--detach", "--force-recreate"); err != nil { + return "", fmt.Errorf("docker up: %w\n"+ + " hint: this environment has no Docker daemon. Pass --local to run the tests "+ + "against mxcli's own runtime instead — no container needed.", err) + } + + fmt.Fprintf(w, "Waiting for test execution (timeout: %s)...\n", timeout) + logOutput, err := captureRuntimeLogs(dockerDir, timeout, w, opts.Verbose) + if err != nil { + return logOutput, fmt.Errorf("runtime execution: %w", err) + } + return logOutput, nil +} + // captureRuntimeLogs tails the docker compose logs, waiting for MXTEST:END or timeout. // Returns the captured log output. func captureRuntimeLogs(dockerDir string, timeout time.Duration, w io.Writer, verbose bool) (string, error) { diff --git a/cmd/mxcli/testrunner/runner_local.go b/cmd/mxcli/testrunner/runner_local.go new file mode 100644 index 000000000..9092c6d62 --- /dev/null +++ b/cmd/mxcli/testrunner/runner_local.go @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "bufio" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// Local test runs deliberately do not share the dev loop's ports or database: +// `mxcli run --local` may well be serving the same project (that is the point of +// the warm loop), and a test run must neither refuse to start because of it nor +// write its fixtures into the database the developer is looking at. +const ( + localTestAppPort = 8081 + localTestAdminPort = 8091 + localTestServePort = 6544 + // localTestDBSuffix is appended to the project's local database name. + localTestDBSuffix = "_test" +) + +// runLocalAndCapture builds the project and boots mxcli's own runtime — no +// Docker — then reads the test runner's output out of the runtime log. +func runLocalAndCapture(opts RunOptions, timeout time.Duration, w io.Writer) (string, error) { + logPath := filepath.Join(filepath.Dir(opts.ProjectPath), ".mxcli", "test-runtime.log") + // The log is appended across runs, so remember where this run starts. + offset := fileSize(logPath) + + fmt.Fprintln(w, "Starting local runtime (no Docker)...") + app, err := docker.StartLocalApp(docker.LocalAppOptions{ + ProjectPath: opts.ProjectPath, + AppPort: localTestAppPort, + AdminPort: localTestAdminPort, + ServePort: localTestServePort, + DB: docker.DBConfig{ + Name: docker.DeriveDBName(opts.ProjectPath) + localTestDBSuffix, + }, + EnsureDB: true, + SkipBuild: opts.SkipBuild, + // The runner reports through an after-startup microflow, so its LOG output + // is produced DURING the start action — before the runtime's own log + // subscriber is attached. What carries it is the JVM console tee, which is + // live from spawn. Verified on 11.12.1; registering the subscriber early + // instead is not an option, the runtime rejects it pre-start with a + // LoggingException. If a future runtime stops echoing to the console the + // failure is loud, not silent: unseen tests are reported as errors. + RuntimeLogPath: logPath, + Stdout: w, + Stderr: w, + }) + if err != nil { + // A failing test IS a failed boot: the generated runner returns false, so + // the runtime's after-startup action fails and `start` reports an error. + // That is a normal test outcome, not a broken run — if the log shows the + // runner reached a verdict, hand it back and let the results speak. (The + // Docker path gets this for free by only ever reading the container log.) + tail := readFrom(logPath, offset) + if runnerReportedVerdict(tail) { + return tail, nil + } + if tail != "" { + return tail, fmt.Errorf("local runtime: %w", err) + } + return "", fmt.Errorf("local runtime: %w", err) + } + defer app.Stop() + + fmt.Fprintf(w, "Waiting for test execution (timeout: %s)...\n", timeout) + return waitForTestLog(logPath, offset, timeout, w, opts.Verbose) +} + +// waitForTestLog polls the runtime log from offset until the run reports a +// terminal marker or the timeout expires, returning everything this run wrote. +// +// Polling rather than following: the after-startup microflow normally completes +// inside the start action, so the output is usually already on disk by the time +// this is called — the loop exists for the case where it is not. +func waitForTestLog(path string, offset int64, timeout time.Duration, w io.Writer, verbose bool) (string, error) { + deadline := time.Now().Add(timeout) + echoed := 0 + + for { + content := readFrom(path, offset) + + if verbose { + lines := splitLines(content) + for ; echoed < len(lines); echoed++ { + fmt.Fprintln(w, lines[echoed]) + } + } + + if done, failMsg := scanTestLog(content); done { + if failMsg != "" { + return content, fmt.Errorf("runtime failed: %s", failMsg) + } + return content, nil + } + + if time.Now().After(deadline) { + return content, fmt.Errorf("timeout after %s waiting for test completion", timeout) + } + time.Sleep(250 * time.Millisecond) + } +} + +// scanTestLog reports whether the log has reached a terminal state, and the +// failure line when the runtime failed rather than the tests completing. The +// markers match the Docker path's, so both modes stop on the same conditions. +func scanTestLog(content string) (done bool, failMsg string) { + for _, line := range splitLines(content) { + switch { + case strings.Contains(line, "Error starting runtime"), + strings.Contains(line, "Critical error"), + strings.Contains(line, "After startup microflow should return a boolean"): + return true, line + case strings.Contains(line, "Successfully ran after-startup-action"), + runnerReportedVerdict(line): + return true, "" + } + } + return false, "" +} + +// runnerReportedVerdict reports whether the test runner got far enough to +// produce results — either it finished, or the runtime said the after-startup +// action failed, which is what a failing test looks like from outside. +func runnerReportedVerdict(content string) bool { + return strings.Contains(content, "MXTEST:END:") || + strings.Contains(content, "after-startup-action failed") || + strings.Contains(content, "After-startup action failed") +} + +func splitLines(s string) []string { + if s == "" { + return nil + } + return strings.Split(strings.TrimRight(s, "\n"), "\n") +} + +// fileSize returns the file's current size, or 0 when it does not exist yet. +func fileSize(path string) int64 { + info, err := os.Stat(path) + if err != nil { + return 0 + } + return info.Size() +} + +// readFrom returns the file's content from offset onward, or "" if unreadable. +func readFrom(path string, offset int64) string { + f, err := os.Open(path) + if err != nil { + return "" + } + defer f.Close() + if _, err := f.Seek(offset, io.SeekStart); err != nil { + return "" + } + var b strings.Builder + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) + for sc.Scan() { + b.WriteString(sc.Text()) + b.WriteByte('\n') + } + return b.String() +} diff --git a/cmd/mxcli/testrunner/runner_local_test.go b/cmd/mxcli/testrunner/runner_local_test.go new file mode 100644 index 000000000..e791a3fe4 --- /dev/null +++ b/cmd/mxcli/testrunner/runner_local_test.go @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// A failing test makes the runner return false, which makes the runtime's +// after-startup action fail, which makes `start` report an error. That is a test +// verdict, not a broken run — the local path must recognise it and let the +// results be parsed rather than aborting with a runtime error. +func TestRunnerReportedVerdict(t *testing.T) { + tests := []struct { + name string + log string + want bool + }{ + {"clean finish", "INFO - MXTEST: MXTEST:END:tests\n", true}, + {"m2ee wording", "The after-startup-action failed with an exception or returned false.\n", true}, + {"runtime log wording", "2026-01-01 ERROR - Core: After-startup action failed.\n", true}, + {"still running", "INFO - MXTEST: MXTEST:RUN:test_1:adds\n", false}, + {"boot died early", "java.lang.OutOfMemoryError\n", false}, + {"empty", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := runnerReportedVerdict(tt.log); got != tt.want { + t.Errorf("runnerReportedVerdict(%q) = %v, want %v", tt.log, got, tt.want) + } + }) + } +} + +func TestScanTestLog(t *testing.T) { + tests := []struct { + name string + log string + wantDone bool + wantFailure bool + }{ + {"end marker", "MXTEST:END:tests\n", true, false}, + {"after-startup success", "Successfully ran after-startup-action\n", true, false}, + {"failing test", "Core: After-startup action failed.\n", true, false}, + {"runtime failure", "Error starting runtime: boom\n", true, true}, + {"non-boolean runner", "After startup microflow should return a boolean\n", true, true}, + {"mid-run", "MXTEST:RUN:test_1:adds\n", false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + done, failMsg := scanTestLog(tt.log) + if done != tt.wantDone { + t.Errorf("done = %v, want %v", done, tt.wantDone) + } + if (failMsg != "") != tt.wantFailure { + t.Errorf("failMsg = %q, want failure=%v", failMsg, tt.wantFailure) + } + }) + } +} + +// The runtime log is appended across runs, so a run must read only what it +// wrote — otherwise the previous run's verdict is reported as this one's. +func TestReadFrom_SkipsEarlierRuns(t *testing.T) { + path := filepath.Join(t.TempDir(), "runtime.log") + previous := "MXTEST:END:old run\n" + if err := os.WriteFile(path, []byte(previous), 0o644); err != nil { + t.Fatal(err) + } + + offset := fileSize(path) + if offset != int64(len(previous)) { + t.Fatalf("offset = %d, want %d", offset, len(previous)) + } + + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + t.Fatal(err) + } + f.WriteString("MXTEST:START:new run\n") + f.Close() + + got := readFrom(path, offset) + if strings.Contains(got, "old run") { + t.Errorf("read included the previous run: %q", got) + } + if !strings.Contains(got, "new run") { + t.Errorf("read missed this run's output: %q", got) + } +} + +func TestFileSize_MissingFileIsZero(t *testing.T) { + if got := fileSize(filepath.Join(t.TempDir(), "absent.log")); got != 0 { + t.Errorf("fileSize of a missing file = %d, want 0", got) + } +} + +func TestWaitForTestLog_ReturnsOnTerminalMarker(t *testing.T) { + path := filepath.Join(t.TempDir(), "runtime.log") + content := "MXTEST:START:s\nMXTEST:PASS:test_1\nMXTEST:END:s\n" + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + got, err := waitForTestLog(path, 0, 2*time.Second, &out, false) + if err != nil { + t.Fatalf("waitForTestLog: %v", err) + } + if !strings.Contains(got, "MXTEST:PASS:test_1") { + t.Errorf("returned log missing the result line: %q", got) + } +} + +func TestWaitForTestLog_TimesOutWithoutMarker(t *testing.T) { + path := filepath.Join(t.TempDir(), "runtime.log") + if err := os.WriteFile(path, []byte("still booting\n"), 0o644); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + got, err := waitForTestLog(path, 0, 300*time.Millisecond, &out, false) + if err == nil { + t.Fatal("expected a timeout error") + } + // The partial log still comes back, so the caller can show what happened. + if !strings.Contains(got, "still booting") { + t.Errorf("timeout dropped the partial log: %q", got) + } +} diff --git a/docs-site/src/migration/validation.md b/docs-site/src/migration/validation.md index 2cec7163b..2df058ccf 100644 --- a/docs-site/src/migration/validation.md +++ b/docs-site/src/migration/validation.md @@ -71,7 +71,7 @@ CALL MICROFLOW Sales.ACT_Order_CalculateTotal ($Order = $Order); ``` ```bash -# Run tests (requires Docker) +# Run tests (Docker, or --local for mxcli's own runtime) mxcli test tests/ -p app.mpr ```