From e4491813d0b266f0c7f4e15c1201fb628140c7d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:55:58 +0000 Subject: [PATCH 01/60] fix(test): read the after-startup setting from the colon-form describe line DESCRIBE SETTINGS now writes `AfterStartupMicroflow: 'Mod.Flow',`. The test runner split only on `=`, kept the whole line as the value, failed to chain the after-startup microflow, and restored the setting to `'AfterStartupMicroflow: ''Mod.Flow'`, leaving the project on MxTest.RegisterEndpoint. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- cmd/mxcli/testrunner/runner.go | 15 ++++++---- cmd/mxcli/testrunner/runner_cleanup_test.go | 32 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index fae3d0ea33..e0ac099c87 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -597,7 +597,7 @@ func getAfterStartup(projectPath string) (string, error) { } for _, line := range strings.Split(string(output), "\n") { - if strings.Contains(line, "AfterStartupMicroflow") { + if strings.HasPrefix(strings.TrimSpace(line), "AfterStartupMicroflow") { return parseSettingValue(line), nil } } @@ -606,7 +606,11 @@ func getAfterStartup(projectPath string) (string, error) { // parseSettingValue extracts the value from one DESCRIBE SETTINGS line, e.g. // -// AfterStartupMicroflow = 'Module.Name', +// AfterStartupMicroflow: 'Module.Name', +// +// or the older `AfterStartupMicroflow = 'Module.Name',`. The value starts after +// the first separator, whichever it is: splitting only on `=` kept the whole +// colon-form line, and the restore wrote the key into the setting's value. // // DESCRIBE SETTINGS separates properties with commas and ends the statement with // a semicolon, so the trailing punctuation must come off *before* the quotes: @@ -615,13 +619,14 @@ func getAfterStartup(projectPath string) (string, error) { // setting (mendixlabs/mxcli#803). func parseSettingValue(line string) string { val := strings.TrimSpace(line) - if _, after, found := strings.Cut(val, "="); found { - val = after + if i := strings.IndexAny(val, ":="); i >= 0 { + val = val[i+1:] } val = strings.TrimSpace(val) val = strings.TrimRight(val, ",;") val = strings.TrimSpace(val) - return strings.Trim(val, "'\"") + val = strings.Trim(val, "'\"") + return strings.ReplaceAll(val, "''", "'") } // quoteMDLString renders a value as an MDL single-quoted literal. Mendix escapes diff --git a/cmd/mxcli/testrunner/runner_cleanup_test.go b/cmd/mxcli/testrunner/runner_cleanup_test.go index 03f5642fa1..ced296bb91 100644 --- a/cmd/mxcli/testrunner/runner_cleanup_test.go +++ b/cmd/mxcli/testrunner/runner_cleanup_test.go @@ -55,6 +55,36 @@ func TestParseSettingValue(t *testing.T) { line: " SomeSetting = 'a=b',", want: "a=b", }, + // DESCRIBE SETTINGS has written `Key: 'value'` since the describe + // rewrite; the `=` cases above are the older output, kept so a project + // described by an older binary still parses. Reading the colon form + // with the `=` parser kept the whole line, and the restore wrote + // `AfterStartupMicroflow: 'AfterStartupMicroflow: ''Mod.Flow'`. + { + name: "colon form, trailing comma (current describe output)", + line: " AfterStartupMicroflow: 'BIA.ASU_SeedDemoData',", + want: "BIA.ASU_SeedDemoData", + }, + { + name: "colon form, trailing semicolon", + line: " AfterStartupMicroflow: 'BIA.ASU_SeedDemoData';", + want: "BIA.ASU_SeedDemoData", + }, + { + name: "colon form, empty value", + line: " AfterStartupMicroflow: '',", + want: "", + }, + { + name: "colon form, value containing a colon is not truncated", + line: " SomeSetting: 'a:b',", + want: "a:b", + }, + { + name: "doubled quote is unescaped", + line: " SomeSetting: 'it''s',", + want: "it's", + }, { name: "no equals sign at all", line: " AfterStartupMicroflow", @@ -77,6 +107,8 @@ func TestParseSettingValue_RoundTripsThroughQuoting(t *testing.T) { " AfterStartupMicroflow = 'MyFirstModule.ASU_Startup',", " AfterStartupMicroflow = 'MyFirstModule.ASU_Startup';", " AfterStartupMicroflow = 'Mod.Flow'", + " AfterStartupMicroflow: 'MyFirstModule.ASU_Startup',", + " AfterStartupMicroflow: 'Mod.Flow';", } { got := quoteMDLString(parseSettingValue(line)) if strings.Count(got, "'") != 2 { From 7e3fc4fe8961025fb5a0ccde05b8c8a01181176e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:55:58 +0000 Subject: [PATCH 02/60] fix(check): a rendered test file no longer warns MDL-V1-SLASH CheckSource closed each test block's wrapper with `END; /`, a `/` the author never wrote, so every test drew an MDL-V1-SLASH note. Records findings for this and the after-startup parse. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .claude/skills/fix-issue/findings/cmd-mxcli.jsonl | 2 ++ cmd/mxcli/testrunner/check_source.go | 2 +- cmd/mxcli/testrunner/check_source_test.go | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 5033f17a65..fd83e3c7dc 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -139,3 +139,5 @@ {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli check`/`exec`/`fmt`/`diff` fail on a script saved by Windows PowerShell 5.1: a UTF-8 BOM gives `line 1:0 token recognition error at: '\\ufeff'` (an invisible character), UTF-16LE gives a token error on almost every character; the same through stdin and in .test.mdl files", "cause": "Every script reader passed the raw file bytes to the lexer, which reads UTF-8 without a BOM; there was no shared reader (fmt, diff, the multi-file check pass, the test runner and EXECUTE SCRIPT each called os.ReadFile on their own)", "fix": "New mdl/srctext.Decode (strip a leading UTF-8 BOM, decode UTF-16LE/BE by BOM); readMDLSource calls it and fmt, diff and parseScriptSet now read through readMDLSource; testrunner.ParseTestFile and EXECUTE SCRIPT call it directly", "insight": "A BOM also hides a `mdl 1;` header from langver.ScanWrittenHeader, so stripping it in the parser alone would have left the language version wrong: decode where the bytes are read, before anything inspects the text. Enumerate the readers (grep os.ReadFile / io.ReadAll(os.Stdin)), not just the one the report names", "issue": "mendixlabs/mxcli#1253", "file": "mdl/srctext/srctext.go; cmd/mxcli/mdlsource.go", "test": "mdl/srctext/srctext_test.go; cmd/mxcli/mdlsource_encoding_test.go"} {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli -p App.mpr -c \"\"` opens the interactive REPL (a generator spawning mxcli with an open stdin hangs at `mdl>`); `-c \"describe entity System.User; describe entity String; describe entity System.FileDocument\"` stops at statement 2 with `module name is required: objects must be created within a module` and the third statement is silently never run", "cause": "Root Run tested `commands != \"\"` to choose -c over the REPL, so an empty flag value was indistinguishable from no flag; the -c path used ExecuteProgram, which returns the first error without its position, and describe entity/association reached findModule(\"\"), whose message is written for the create path", "fix": "`cmd.Flags().Changed(\"command\")` selects the one-liner path; runCommandLine (cmd/mxcli/oneliner.go) refuses empty input, reports `statement N of M` and how many later statements were not run (via new Executor.ExecuteProgramReportingStop), and takes --continue-on-error like exec; execDescribe names an unqualified entity/association name", "insight": "A flag's zero value is not its absence: use Changed() whenever an empty value must mean something other than not given. Decided semantics: -c is fail-fast like exec (a later statement may depend on an earlier one), but a stop is never silent", "issue": "mendixlabs/mxcli#1218", "file": "cmd/mxcli/oneliner.go; cmd/mxcli/main.go; mdl/executor/executor.go; mdl/executor/executor_query.go", "test": "cmd/mxcli/oneliner_test.go; mdl/executor/describe_unqualified_name_test.go"} {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli lsp --stdio` exits rc=2 with `panic: only file URIs are supported, got mendix-mdl` on textDocument/didOpen of a `mendix-mdl:` virtual document (the VS Code extension's describe previews); VS Code restarts it, it crashes again, and after 5 crashes it stops restarting the server", "cause": "checkableDocument (diagnostics) and CodeAction called go.lsp.dev/uri URI.Filename(), which panics on any scheme but file, to decide whether the document is a .test.mdl", "fix": "documentPath(uri) returns Filename() only for file: URIs and the URI's path component otherwise; runSemanticCheck (which shells out `mxcli check `) skips non-file documents; virtual documents are still diagnosed in memory", "insight": "Third-party helpers that panic on unexpected input are a crash path in a long-running server; every URI an LSP client sends is untrusted shape. A test with a non-file URI plus a file-URI control (same diagnostics) proves the virtual case is handled, not skipped", "issue": "mendixlabs/mxcli#1245", "file": "cmd/mxcli/lsp_helpers.go (documentPath, isFileURI); cmd/mxcli/lsp_diagnostics.go; cmd/mxcli/lsp_language.go", "test": "cmd/mxcli/lsp_virtual_uri_test.go"} +{"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`mxcli test --local` fails to chain the project's after-startup microflow, then cleanup restores `AfterStartupMicroflow: 'AfterStartupMicroflow: ''Mod.Flow'` and the project is left pointing at `MxTest.RegisterEndpoint`", "cause": "`parseSettingValue` scraped DESCRIBE SETTINGS text and split only on `=`; the describe rewrite changed the output to `Key: 'value'`, so the whole line was kept as the value. The #803 tests used the old `=` lines only and stayed green against output describe no longer emits", "file": "`cmd/mxcli/testrunner/runner.go` (`parseSettingValue`, `getAfterStartup`)", "insight": "A parser of another command's human-readable output breaks silently when that output changes; the table test must carry the line the producer emits *today*. Split on the first `:` or `=`, unescape `''`, and match the key as a line prefix. Longer term, read the setting through the backend instead of scraping describe", "refs": []} +{"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`mxcli check tests/X.test.mdl` (and the VS Code LSP) warns MDL-V1-SLASH once per test, on the doc-comment line, although the skill says a test file takes no `mdl 1;` header", "cause": "`CheckSource` renders each block as a microflow and closed the wrapper with `END; /`; the rendering is parsed headerless, so the visitor recorded a V1 slash note on a `/` mxcli itself wrote. The author's `/` separators are blanked and never reach the parser", "file": "`cmd/mxcli/testrunner/check_source.go`", "insight": "Generated MDL must be canonical under every language version, or the diagnostics blame the author's file. `TestCheckSourceWrapperIsCanonical` asserted no Deprecations but not LanguageNotes; it now asserts both", "refs": []} diff --git a/cmd/mxcli/testrunner/check_source.go b/cmd/mxcli/testrunner/check_source.go index 02021ec435..32e992f634 100644 --- a/cmd/mxcli/testrunner/check_source.go +++ b/cmd/mxcli/testrunner/check_source.go @@ -95,7 +95,7 @@ func CheckSource(content, path string) (CheckedSource, error) { // A void microflow needs no RETURN, so the wrapper is two fragments and // the body between them is exactly what the author typed. place(out, first-1, fmt.Sprintf("CREATE OR MODIFY MICROFLOW %s.%s () BEGIN", mxTestModule, checkFlowName(tc, i))) - place(out, first+len(body), "END; /") + place(out, first+len(body), "END;") } return CheckedSource{MDL: strings.Join(out, "\n"), Problems: problems}, nil diff --git a/cmd/mxcli/testrunner/check_source_test.go b/cmd/mxcli/testrunner/check_source_test.go index caa2491747..cc6aeec48f 100644 --- a/cmd/mxcli/testrunner/check_source_test.go +++ b/cmd/mxcli/testrunner/check_source_test.go @@ -135,6 +135,14 @@ func TestCheckSourceWrapperIsCanonical(t *testing.T) { t.Fatalf("the rendering records %d deprecated spelling(s), first %s on line %d:\n%s", len(prog.Deprecations), prog.Deprecations[0].Code, prog.Deprecations[0].Line, got.MDL) } + // The rendering is parsed headerless, so any construct a later language + // version changes is reported against the author's file. The wrapper once + // closed each block with `END; /`, and every test in the file drew an + // MDL-V1-SLASH warning on a `/` the author never wrote. + if len(prog.LanguageNotes) != 0 { + t.Fatalf("the rendering records %d language note(s), first %s on line %d:\n%s", + len(prog.LanguageNotes), prog.LanguageNotes[0].Code, prog.LanguageNotes[0].Line, got.MDL) + } } // A .test.md block's body sits on the line after its doc comment, as in a From af17ac998f8e8747d1b740a93c0385a7cc0ebaaf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:57:35 +0000 Subject: [PATCH 03/60] fix(association): do not reconcile System's access rules An association to a System entity reconciled System's domain model, which is virtual and has no stored unit, so the create failed with "no such file" after the association was already written. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + mdl/executor/cmd_associations.go | 8 ++- .../cmd_associations_system_reconcile_test.go | 59 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 mdl/executor/cmd_associations_system_reconcile_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index b93a41eb07..6c3e9709f2 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -800,3 +800,4 @@ {"date": "2026-10-01", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#175: `call workflow … on error continue` passed check and exec, then mx check: CE6035 at Call workflow activity. On 11.14.0 only continue fails: no clause, rollback and both custom handlers build.", "cause": "continueUnsupportedOn did not list call workflow, and adapters.StatementErrorHandling — a hand-kept list of statements carrying a clause — did not list CallWorkflowStmt (nor the REST/other workflow statements), so MDL076 could not even see the clause. MDL076 was also check-only: exec without the pre-check (-c, REPL, --no-check) wrote it.", "file": "mdl/executor/validate_microflow_error_handling.go; mdl/exprcheck/adapters/adapter_scope.go; mdl/executor/validate.go", "fix": "call workflow in continueUnsupportedOn; StatementErrorHandling falls back to reading any statement's ErrorHandling field by reflection (getErrorHandling delegates to it); MDL076 is exec-enforced.", "test": "TestMDL076_ReportsContinueOnCallWorkflow, TestMDL076_CallWorkflowAcceptsTheOtherClauses, TestMDL076_IsExecEnforced", "insight": "Three lists of 'statements with an ON ERROR clause' existed; each had drifted. A rule keyed on such a list silently does nothing for a statement the list forgot."} {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#571 / mendixlabs/mxcli#1206: `create published rest service` wrote only the path's {name} placeholders as operation parameters, each a String: every query and body microflow parameter failed mx check with CE0350, an Integer {id} with CE6539. `import mapping` / `export mapping` / `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on an object-returning microflow). Executing describe of a Studio Pro service (TestApp Services.OrdersRestApi) reported 'Modified' and broke a 0-error app with 5 errors: mappings cleared, Integer path params retyped String, body param dropped, Commit No->Yes, Basic+Session authentication turned off", "cause": "publishedRestOperationToGen built parameters from the path alone and wrote ExportMapping/ImportMapping \"\" and Commit \"Yes\" as constants; the reader never read parameters, mappings or commit, so describe could not print them and ALTER (which rewrites every operation) lost them too; the service writer also emitted constants for AuthenticationTypes / AuthenticationMicroflow / CorsConfiguration / Documentation / PublicDocumentation with no carry; Resources and operation Parameters were registered with list marker 2 where Studio Pro writes 3", "file": "mdl/executor/cmd_published_rest.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/export_level_carry.go, mdl/visitor/visitor_rest.go, model/types.go", "fix": "the executor derives operation parameters from the microflow as Studio Pro does (path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, else Query; the microflow parameter's type), merged over the stored parameters per bound microflow parameter so a header/renamed/described parameter survives; mappings and commit flow AST -> model -> BSON and back, describe prints them (commit when not Yes) and notes parameters MDL cannot state; an unknown commit value is refused at exec and by check (MDL-REST03); create or modify carries summary/documentation/object handling of the restated operation; UpdatePublishedRestService carries the stored service-level keys MDL cannot state (keepStoredTopLevel); list markers measured from TestApp", "test": "mdl/executor/cmd_published_rest_params_test.go; mdl/backend/modelsdk/published_rest_write_test.go TestCreatePublishedRestService_WritesParametersAndBindings, TestWithStoredTopLevel; mdl/roundtrip TestTestAppRoundTrip/published_rest_service_Services.OrdersRestApi (allowlist entry struck); mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl (TestApp copy: old binary 16 mx check errors, fixed 0, describe->exec Unchanged twice)", "insight": "A clause that parses and is then ignored is worse than a parse error: the grammar advertised import/export mapping for months while the writer hard-coded them empty. The fastest witness was the round-trip harness's own allowlist entry for the one Studio Pro published REST service in TestApp: removing it printed the whole loss set (bindings, parameter types, markers, authentication) in one diff. Studio Pro's metamodel (ped_get_schema over the MCP tunnel) gave the enum values and defaults: Commit defaults to No there, while mxcli keeps writing Yes when the clause is absent so existing scripts do not churn, and describe prints commit whenever it is not Yes."} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "Nightly, Mendix 10.24 only: `TestMxCheck_DoctypeScripts/24-workflow-examples.mdl/modelsdk` → `skipped 481 version-gated lines` then `Execution error: entity 'WFTest.OrderContext' not found for parameter 'OrderContext'` — the same failure TestFilterByVersion_FileBaselineSurvivesAny was written for, back again with that test green", "cause": "`filterByVersion` treats a `-- @version:` directive as the file's floor only if no statement precedes it. #901 put `mdl 1;` on line 1 of every doctype script, above 24's `-- @version: 11.0+`; the header counted as a statement, the floor was lost, and PART H's `-- @version: any` re-enabled a section whose fixtures (WFTest.OrderContext, 11.0+) had been skipped", "file": "`mdl/executor/roundtrip_doctype_test.go` (filterByVersion)", "insight": "The language header is not a statement: `langver.IsHeaderLine` excludes it. The earlier regression test used a synthetic script without a header, so a corpus-wide header migration could not trip it — the new guard also runs filterByVersion on the real 24-workflow-examples.mdl. When a script-format migration lands, re-run the line-oriented tooling that reads those scripts (version gating, skip lists) against the real files, not synthetic ones. Repro: `MX_BINARY=~/.mxcli/mxbuild/10.24.24.119349/modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts/24-workflow ./mdl/executor/` (fails with exactly 481 skipped lines without the fix, 523 with it)", "refs": ["#901"]} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`create association M.X_Task from M.X to System.WorkflowUserTask` writes the association, then fails with `failed to reconcile access rules of module System: open …/00000000-0000-0000-0000-000000000002.mxunit: no such file`; the rest of the script is aborted. A re-run with `create or modify` is clean", "cause": "`reconcileModuleAccess` reconciles the TO end's module for a cross-module association; for System that is the virtual domain model, which has no stored unit to load. The `create or modify` re-run returns before reconciling, which hid it", "file": "`mdl/executor/cmd_associations.go` (`reconcileModuleAccess`)", "insight": "System is a module with a domain model but no unit — any per-module write sweep must skip it (`isBuiltinModuleEntity`). The alter-owner path reaches the same function", "refs": []} diff --git a/mdl/executor/cmd_associations.go b/mdl/executor/cmd_associations.go index 1b19188197..69bfa520eb 100644 --- a/mdl/executor/cmd_associations.go +++ b/mdl/executor/cmd_associations.go @@ -391,8 +391,14 @@ func reconcileAfterAssociationAlter(ctx *ExecContext, s *ast.AlterAssociationStm // reconcileModuleAccess reconciles one module's entity access rules and tracks // its domain model as modified. A module that cannot be found (the TO end of a -// cross-module association named by a stale reference) has nothing to fix. +// cross-module association named by a stale reference) has nothing to fix, and +// neither does System: its domain model is virtual, not a stored unit, so the +// reconcile failed with "no such file" after an association to a System entity +// had already been written — and its rules cannot be changed by a project. func reconcileModuleAccess(ctx *ExecContext, moduleName, why string) error { + if isBuiltinModuleEntity(moduleName) { + return nil + } mod, err := findModule(ctx, moduleName) if err != nil || mod == nil { return nil diff --git a/mdl/executor/cmd_associations_system_reconcile_test.go b/mdl/executor/cmd_associations_system_reconcile_test.go new file mode 100644 index 0000000000..44a1b83be2 --- /dev/null +++ b/mdl/executor/cmd_associations_system_reconcile_test.go @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// `create association M.Thing_Task from M.Thing to System.WorkflowUserTask` +// wrote the association, then reconciled the TO module's access rules. System's +// domain model is virtual — no unit is stored for it — so the reconcile failed +// with "no such file" and aborted the rest of the script. System's rules cannot +// be changed by a project anyway; there is nothing to reconcile. +func TestReconcileModuleAccess_SkipsSystem(t *testing.T) { + ctx, reconciled := reconcileRecordingAssocFixture(t, false) + mb := ctx.Backend.(*mock.MockBackend) + + mods, _ := mb.ListModulesFunc() + system := mkModule("System") + systemDM := &domainmodel.DomainModel{ + BaseElement: model.BaseElement{ID: nextID("dm")}, + ContainerID: system.ID, + Entities: []*domainmodel.Entity{mkEntity(system.ID, "WorkflowUserTask")}, + } + all := append(append([]*model.Module(nil), mods...), system) + mb.ListModulesFunc = func() ([]*model.Module, error) { return all, nil } + inner := mb.GetDomainModelFunc + mb.GetDomainModelFunc = func(id model.ID) (*domainmodel.DomainModel, error) { + if id == system.ID { + return systemDM, nil + } + return inner(id) + } + ctx.Cache = nil + withHierarchy(mkHierarchy(all...))(ctx) + mb.ReconcileMemberAccessesFunc = func(_ model.ID, moduleName string) (int, error) { + *reconciled = append(*reconciled, moduleName) + if moduleName == "System" { + return 0, fmt.Errorf("open mprcontents/…/00000000-0000-0000-0000-000000000002.mxunit: no such file or directory") + } + return 0, nil + } + + assertNoError(t, reconcileModuleAccess(ctx, "System", "for new association")) + if len(*reconciled) != 0 { + t.Fatalf("reconciled %v; System's domain model is not stored and must be skipped", *reconciled) + } + + // Control: an ordinary module is still reconciled. + assertNoError(t, reconcileModuleAccess(ctx, "M", "for new association")) + if len(*reconciled) != 1 || (*reconciled)[0] != "M" { + t.Fatalf("reconciled %v, want [M]", *reconciled) + } +} From 0ffdaf6c08b0c154844f8b6d135ac0915262f381 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:01:01 +0000 Subject: [PATCH 04/60] fix(workflow): report an elided rewrite as Unchanged create or modify workflow printed "Created workflow" on every run, including in-place updates whose write was elided. It now reports through ReportMutation like every other document type. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + mdl/executor/cmd_workflows_write.go | 8 ++- mdl/executor/workflow_report_mutation_test.go | 53 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 mdl/executor/workflow_report_mutation_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 6c3e9709f2..2aec360845 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -801,3 +801,4 @@ {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#571 / mendixlabs/mxcli#1206: `create published rest service` wrote only the path's {name} placeholders as operation parameters, each a String: every query and body microflow parameter failed mx check with CE0350, an Integer {id} with CE6539. `import mapping` / `export mapping` / `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on an object-returning microflow). Executing describe of a Studio Pro service (TestApp Services.OrdersRestApi) reported 'Modified' and broke a 0-error app with 5 errors: mappings cleared, Integer path params retyped String, body param dropped, Commit No->Yes, Basic+Session authentication turned off", "cause": "publishedRestOperationToGen built parameters from the path alone and wrote ExportMapping/ImportMapping \"\" and Commit \"Yes\" as constants; the reader never read parameters, mappings or commit, so describe could not print them and ALTER (which rewrites every operation) lost them too; the service writer also emitted constants for AuthenticationTypes / AuthenticationMicroflow / CorsConfiguration / Documentation / PublicDocumentation with no carry; Resources and operation Parameters were registered with list marker 2 where Studio Pro writes 3", "file": "mdl/executor/cmd_published_rest.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/export_level_carry.go, mdl/visitor/visitor_rest.go, model/types.go", "fix": "the executor derives operation parameters from the microflow as Studio Pro does (path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, else Query; the microflow parameter's type), merged over the stored parameters per bound microflow parameter so a header/renamed/described parameter survives; mappings and commit flow AST -> model -> BSON and back, describe prints them (commit when not Yes) and notes parameters MDL cannot state; an unknown commit value is refused at exec and by check (MDL-REST03); create or modify carries summary/documentation/object handling of the restated operation; UpdatePublishedRestService carries the stored service-level keys MDL cannot state (keepStoredTopLevel); list markers measured from TestApp", "test": "mdl/executor/cmd_published_rest_params_test.go; mdl/backend/modelsdk/published_rest_write_test.go TestCreatePublishedRestService_WritesParametersAndBindings, TestWithStoredTopLevel; mdl/roundtrip TestTestAppRoundTrip/published_rest_service_Services.OrdersRestApi (allowlist entry struck); mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl (TestApp copy: old binary 16 mx check errors, fixed 0, describe->exec Unchanged twice)", "insight": "A clause that parses and is then ignored is worse than a parse error: the grammar advertised import/export mapping for months while the writer hard-coded them empty. The fastest witness was the round-trip harness's own allowlist entry for the one Studio Pro published REST service in TestApp: removing it printed the whole loss set (bindings, parameter types, markers, authentication) in one diff. Studio Pro's metamodel (ped_get_schema over the MCP tunnel) gave the enum values and defaults: Commit defaults to No there, while mxcli keeps writing Yes when the clause is absent so existing scripts do not churn, and describe prints commit whenever it is not Yes."} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "Nightly, Mendix 10.24 only: `TestMxCheck_DoctypeScripts/24-workflow-examples.mdl/modelsdk` → `skipped 481 version-gated lines` then `Execution error: entity 'WFTest.OrderContext' not found for parameter 'OrderContext'` — the same failure TestFilterByVersion_FileBaselineSurvivesAny was written for, back again with that test green", "cause": "`filterByVersion` treats a `-- @version:` directive as the file's floor only if no statement precedes it. #901 put `mdl 1;` on line 1 of every doctype script, above 24's `-- @version: 11.0+`; the header counted as a statement, the floor was lost, and PART H's `-- @version: any` re-enabled a section whose fixtures (WFTest.OrderContext, 11.0+) had been skipped", "file": "`mdl/executor/roundtrip_doctype_test.go` (filterByVersion)", "insight": "The language header is not a statement: `langver.IsHeaderLine` excludes it. The earlier regression test used a synthetic script without a header, so a corpus-wide header migration could not trip it — the new guard also runs filterByVersion on the real 24-workflow-examples.mdl. When a script-format migration lands, re-run the line-oriented tooling that reads those scripts (version gating, skip lists) against the real files, not synthetic ones. Repro: `MX_BINARY=~/.mxcli/mxbuild/10.24.24.119349/modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts/24-workflow ./mdl/executor/` (fails with exactly 481 skipped lines without the fix, 523 with it)", "refs": ["#901"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`create association M.X_Task from M.X to System.WorkflowUserTask` writes the association, then fails with `failed to reconcile access rules of module System: open …/00000000-0000-0000-0000-000000000002.mxunit: no such file`; the rest of the script is aborted. A re-run with `create or modify` is clean", "cause": "`reconcileModuleAccess` reconciles the TO end's module for a cross-module association; for System that is the virtual domain model, which has no stored unit to load. The `create or modify` re-run returns before reconciling, which hid it", "file": "`mdl/executor/cmd_associations.go` (`reconcileModuleAccess`)", "insight": "System is a module with a domain model but no unit — any per-module write sweep must skip it (`isBuiltinModuleEntity`). The alter-owner path reaches the same function", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`exec` prints `Created workflow: M.X` on every re-run of a `create or modify workflow`, even when `mxcli diff` says nothing would be written", "cause": "`execCreateWorkflow` printed a fixed `Created workflow:` line for both the create and the in-place update branch, bypassing `ReportMutation`, so write elision never surfaced as `Unchanged`", "file": "`mdl/executor/cmd_workflows_write.go` (`execCreateWorkflow`)", "insight": "Every mutating handler must report through `ctx.ReportMutation` (Created/Modified, downgraded to Unchanged on elision); a hand-rolled Fprintf is what makes a no-op look like churn", "refs": []} diff --git a/mdl/executor/cmd_workflows_write.go b/mdl/executor/cmd_workflows_write.go index c309543c92..f42ff05ce0 100644 --- a/mdl/executor/cmd_workflows_write.go +++ b/mdl/executor/cmd_workflows_write.go @@ -267,7 +267,13 @@ func execCreateWorkflow(ctx *ExecContext, s *ast.CreateWorkflowStmt) error { } invalidateHierarchy(ctx) - fmt.Fprintf(ctx.Output, "Created workflow: %s.%s\n", s.Name.Module, s.Name.Name) + // Through ReportMutation, like every other document: a rewrite whose unit + // write was elided says "Unchanged", not "Created", on every re-run. + verb := "Created" + if existingID != "" { + verb = "Modified" + } + ctx.ReportMutation(verb, "workflow: %s.%s", s.Name.Module, s.Name.Name) return nil } diff --git a/mdl/executor/workflow_report_mutation_test.go b/mdl/executor/workflow_report_mutation_test.go new file mode 100644 index 0000000000..a0b6e1540d --- /dev/null +++ b/mdl/executor/workflow_report_mutation_test.go @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" + "github.com/mendixlabs/mxcli/sdk/workflows" +) + +// Re-running `create or modify workflow` printed "Created workflow: …" on every +// run, for a rewrite that changed nothing and whose unit write was elided. Every +// other document type reports through ReportMutation and says "Unchanged". +func TestCreateOrModifyWorkflow_ReportsWhatHappened(t *testing.T) { + for _, tc := range []struct { + name string + written int + want string + }{ + {"elided rewrite", 0, "Unchanged workflow: MyModule.Approve"}, + {"rewrite that landed (control)", 1, "Modified workflow: MyModule.Approve"}, + } { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("MyModule") + stored := storedNamedWorkflow(mod) + h := mkHierarchy(mod) + withContainer(h, stored.ContainerID, mod.ID) + cb := &countingBackend{} + cb.MockBackend = &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListWorkflowsFunc: func() ([]*workflows.Workflow, error) { return []*workflows.Workflow{stored}, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{mkPage(mod.ID, "P")}, nil }, + UpdateWorkflowFunc: func(*workflows.Workflow) error { + cb.offer(1, tc.written) + return nil + }, + } + ctx, out := newMockCtx(t, withHierarchy(h)) + ctx.Backend = cb + prog := parseMDL(t, namedWorkflowRewrite) + assertNoError(t, execCreateWorkflow(ctx, prog.Statements[0].(*ast.CreateWorkflowStmt))) + if got := out.String(); !strings.Contains(got, tc.want) { + t.Fatalf("output %q, want it to contain %q", got, tc.want) + } + }) + } +} From feb41c8f173b6334954841987fa84eebe788305e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:03:57 +0000 Subject: [PATCH 05/60] fix(grammar): a grant member list accepts keyword-named attributes entityMemberName took only IDENTIFIER or a quoted name, so attributes named Region, Status, Title, Value or Date could be declared bare but not granted. It now includes keyword, as attributeName does. The keyword-hint tests move to a workflow activity name, which still rejects bare keywords. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-grammar.jsonl | 1 + mdl/grammar/domains/MDLSecurity.g4 | 4 +++- mdl/visitor/reserved_keyword_hint_test.go | 11 ++++++--- mdl/visitor/visitor_security_test.go | 23 +++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/.claude/skills/fix-issue/findings/mdl-grammar.jsonl b/.claude/skills/fix-issue/findings/mdl-grammar.jsonl index d8f8d9fcfd..08dcb28501 100644 --- a/.claude/skills/fix-issue/findings/mdl-grammar.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-grammar.jsonl @@ -68,3 +68,4 @@ {"area": "mdl/grammar", "date": "2026-09-30", "symptom": "`create or modify constant M.DbPassword type string default '' PRIVATE;` -> `line 22:80 no viable alternative at input 'PRIVATE'` after upgrading past v0.24.0; `fmt --upgrade` failed with the same error because it must parse first. 9 credential constants in mxcli-formula1; the database-connections skill had taught the word.", "cause": "No grammar rule ever had PRIVATE. In v0.24.0 (and nightly 8c227f46) `helpStatement: IDENTIFIER (helpTopicWord …)*` was a catch-all and `;` was optional, so `… default '' private;` parsed as TWO statements: the constant, then a help statement `private` that the visitor built nothing for (`check` said `Syntax OK (2 statements)`). R7's IsHelpWord predicate (#755, bf0d1d38; the gated b518ae72 was reverted by dd977eea) closed the catch-all, turning the silent no-op into a parse error with no registry entry.", "file": "`mdl/grammar/domains/MDLDomainModel.g4` (constantOption `{IsPrivateWord(...)}? IDENTIFIER /* @alias MDL-DEPR138 */`), `mdl/grammar/MDLParser.g4` (IsPrivateWord), `mdl/deprecation/deprecation.go` (ConstantPrivate, RemovedIn 1), `mdl/visitor/visitor_enumeration.go` (record + delete fix), `mdl/visitor/visitor_deprecations.go` (recordDeprecation refuses at RemovedIn), tests `mdl/visitor/visitor_constant_private_test.go`, `mdl/upgrade/constant_private_test.go`, example `mdl-examples/deprecated-aliases/bug-tests--865-constant-private.mdl`", "insight": "To find what an old catch-all silently accepted, do not diff grammars — the word was in neither. Dump the OLD parse tree (`ToStringTree`) for the failing line; it showed `(helpStatement private)`. Then audit the class by walking every error-free old parse of a corpus for helpStatement nodes whose word is not help/exit/quit: over the repo's skills/docs/examples at 8c227f46 and all of mxcli-formula1, the only such word in a clean parse was PRIVATE (after a constant), so a targeted alias is the whole fix, not re-opening the catch-all. Match the word by a semantic predicate on IDENTIFIER, not a new keyword, so `private` stays usable as a name. Divergence to know: the old catch-all also swallowed words AFTER private (`private exposed to client` dropped the exposure); the alias does not.", "refs": ["ako/mxcli#865", "ako/mxcli#755", "ako/mxcli#714"]} {"area": "mdl/grammar", "date": "2026-10-01", "symptom": "`fmt --upgrade --header` refused `$Ordered = sort($Rows, Position);` with `MDL-V1-LIST: the operand is not a variable (a nested call or an expression)` and left the whole file at mdl 0 (2 mxcli-formula1 files); any keyword attribute (Position, Status, Type, Date, Value, Title, Caption, Content, Index) did it, and `sort($L, Status desc)` did not parse at all.", "cause": "The call form's `sortSpec` took only `IDENTIFIER | QUOTED_IDENTIFIER`, while the statement form's `listSortItem` took `identifierOrKeyword`. With a keyword attribute the listOperationStatement alternative failed and the line fell through to setStatement as a generic function call; mdl 0 still built the sort from it (buildListOrAggregateStatement), but the upgrade fix (setCallFix -> singleListCall) found no ListOperationContext and reported a nested call.", "file": "`mdl/grammar/domains/MDLMicroflow.g4` (sortSpec: identifierOrKeyword (ASC|DESC)?), `mdl/visitor/visitor_microflow_actions.go` (buildSortSpecList), `mdl/visitor/visitor_microflow_expression.go` (sort spec args); tests `mdl/upgrade/gated_test.go` TestUpgrade_KeywordAttributeNames, `mdl/visitor/visitor_microflow_sort_quoted_test.go` TestUnquotedKeywordSortAttribute", "insight": "A misleading upgrade reason ('not a variable') was a grammar asymmetry between a call form and its statement form: when one rule falls through to the generic expression path, the upgrader sees a function call and loses the structure. Compare the two forms' operand rules side by side. Proven with a control binary: `check` over 741 example/doc/skill scripts identical, and `fmt --upgrade --header` over 172 rehearsal scripts differs in exactly the two formula1 files. find/filter by member and sum/min/max over `$L.Keyword` were already fine.", "refs": ["ako/mxcli#889", "ako/mxcli#714"]} {"date": "2026-10-01", "area": "mdl/grammar", "symptom": "mendixlabs/mxcli#992: `@anchor(true: (to: top))` — the per-case form DESCRIBE emits, with one side — parsed, passed check and exec, and left the true edge on the default sides. `@anchor(true: (from: right, to: top))` worked. `@curve(true: …)` on a NANOFLOW passed check and exec and was dropped (on a microflow MDL060 refused it).", "cause": "annotationParam listed `annotationValue | annotationParenValue`; `(to: top)` also matches annotationValue's expression alternative, as `to : top` — `:` is Mendix's division operator — and ANTLR took the first alternative, so the nested-anchor reader found no paren value. The visitor then skipped anything it could not use. Nanoflows never ran the annotation rules (ValidateNanoflow ran only MDL044).", "file": "mdl/grammar/domains/MDLSettings.g4 (annotationParam); mdl/visitor/visitor_microflow_statements.go (parseAnchorAnnotation)", "fix": "annotationParenValue before annotationValue; parseAnchorAnnotation records every parameter it cannot use in InvalidAnchors, refused as MDL092 at check and exec; ValidateNanoflow runs checkUnknownAnnotations over the whole body.", "test": "TestAnchorAnnotation_SplitBranchWithOneSide, TestAnchorAnnotation_RecordsWhatItCannotUse, TestSplitBranch_OneSidedAnchorReachesTheEdge, TestAnchorParameterItCannotUseIsRefused", "insight": "A two-key test case hid the one-key failure: `(from: x, to: y)` cannot be an expression (comma), `(to: y)` can. Dump the parse tree (ToStringTree) for the exact failing text before reading the visitor."} +{"area": "mdl/grammar", "date": "2026-10-02", "symptom": "`grant read on M.E (FullName, Region)` fails with \"mismatched input 'Region'\"; `\"Region\"` works. Same for Status, Title, Value, Date, Label, … — names `create entity` accepts unquoted", "cause": "`entityMemberName` in MDLSecurity.g4 was `IDENTIFIER | QUOTED_IDENTIFIER`, the one name rule without `| keyword`; `Region` lexes as SCROLLREGION. An earlier fix answered the error with a \"quote it\" hint instead of accepting the name", "file": "`mdl/grammar/domains/MDLSecurity.g4` (`entityMemberName`)", "insight": "A name a statement can declare must be usable everywhere it is referenced; compare the slot with `attributeName` before adding hints. The keyword hint's tests moved to `workflowActivityName`, which still rejects bare keywords", "refs": []} diff --git a/mdl/grammar/domains/MDLSecurity.g4 b/mdl/grammar/domains/MDLSecurity.g4 index 27d00425f1..0ef88ca1b7 100644 --- a/mdl/grammar/domains/MDLSecurity.g4 +++ b/mdl/grammar/domains/MDLSecurity.g4 @@ -218,8 +218,10 @@ entityAccessRight // Member (attribute / association) name in a READ/WRITE list. Accepts a quoted // identifier so members whose name is a reserved word can be escaped, e.g. -// READ ("Order", Status). +// READ ("Order", Status), and a keyword, as attributeName does — without it an +// attribute named Region, Status or Title could be declared but not granted. entityMemberName : IDENTIFIER | QUOTED_IDENTIFIER + | keyword ; diff --git a/mdl/visitor/reserved_keyword_hint_test.go b/mdl/visitor/reserved_keyword_hint_test.go index 16e0b0fd43..f9cb39c2f8 100644 --- a/mdl/visitor/reserved_keyword_hint_test.go +++ b/mdl/visitor/reserved_keyword_hint_test.go @@ -13,11 +13,16 @@ import ( "github.com/mendixlabs/mxcli/mdl/types" ) -const grantWith = `grant Administration.Administrator on M.Doc ( read (%s) );` +// keywordSlot is a name position that still takes only an identifier, so a bare +// keyword there is a parse error and draws the hint. The grant member list this +// used to use now accepts keywords, as attribute names do, which is the better +// fix where it applies; the hint still serves the slots that do not. +const keywordSlot = "create workflow M.W\nbegin\n user task %s 'Review'\n page M.P;\nend workflow;" + func hintFor(t *testing.T, name string) string { t.Helper() - _, errs := Build(strings.Replace(grantWith, "%s", name, 1)) + _, errs := Build(strings.Replace(keywordSlot, "%s", name, 1)) if len(errs) == 0 { t.Fatalf("expected a parse error for %q used bare", name) } @@ -41,7 +46,7 @@ func TestQuotingActuallyParses(t *testing.T) { // The control, and the one that makes the advice above honest: a hint that // recommends quoting is only worth printing if the quoted form parses. Without // this, the test above would pass against a suggestion that does not work. - if _, errs := Build(strings.Replace(grantWith, "%s", `"Title"`, 1)); len(errs) != 0 { + if _, errs := Build(strings.Replace(keywordSlot, "%s", `"Title"`, 1)); len(errs) != 0 { t.Fatalf("the hint recommends quoting, but the quoted form does not parse: %v", errs) } } diff --git a/mdl/visitor/visitor_security_test.go b/mdl/visitor/visitor_security_test.go index f07f1a27df..440717b87e 100644 --- a/mdl/visitor/visitor_security_test.go +++ b/mdl/visitor/visitor_security_test.go @@ -3,6 +3,7 @@ package visitor import ( + "strings" "testing" "github.com/mendixlabs/mxcli/mdl/ast" @@ -254,6 +255,28 @@ func TestGrantEntityAccess_QuotedMembers(t *testing.T) { } } +// A member list took only IDENTIFIER or a quoted name, so any attribute whose +// name lexes as a keyword — Region, Status, Title, Value, Date — was a parse +// error ("mismatched input 'Region'") although `create entity` accepts it +// unquoted. Every other name rule in the grammar includes `keyword`. +func TestGrantEntityAccess_KeywordMembers(t *testing.T) { + input := `GRANT MyModule.Admin ON MyModule.Engineer (READ (FullName, Region, Status, Title), WRITE (Value, Date));` + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("Parse error: %v", errs) + } + stmt := prog.Statements[0].(*ast.GrantEntityAccessStmt) + want := [][]string{{"FullName", "Region", "Status", "Title"}, {"Value", "Date"}} + if len(stmt.Rights) != 2 { + t.Fatalf("expected 2 rights, got %d", len(stmt.Rights)) + } + for i, w := range want { + if strings.Join(stmt.Rights[i].Members, ",") != strings.Join(w, ",") { + t.Errorf("right %d members %v, want %v", i, stmt.Rights[i].Members, w) + } + } +} + func TestGrantEntityAccess_MultipleRoles(t *testing.T) { input := `GRANT MyModule.Admin, MyModule.Editor ON MyModule.Customer (READ *);` prog, errs := Build(input) From 65c1da3fd28cc289519417e9d83aeda592257c73 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:05:20 +0000 Subject: [PATCH 06/60] fix(page): qualify an inherited CaptionAttribute with its declaring entity CaptionAttribute was qualified with the context entity, so a caption on an attribute inherited from Administration.Account was stored against the specialization and mxbuild failed with CE1613. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + mdl/executor/widget_engine.go | 5 ++- .../widget_engine_caption_inherited_test.go | 37 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 mdl/executor/widget_engine_caption_inherited_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 2aec360845..37f72b8d6d 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -802,3 +802,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "Nightly, Mendix 10.24 only: `TestMxCheck_DoctypeScripts/24-workflow-examples.mdl/modelsdk` → `skipped 481 version-gated lines` then `Execution error: entity 'WFTest.OrderContext' not found for parameter 'OrderContext'` — the same failure TestFilterByVersion_FileBaselineSurvivesAny was written for, back again with that test green", "cause": "`filterByVersion` treats a `-- @version:` directive as the file's floor only if no statement precedes it. #901 put `mdl 1;` on line 1 of every doctype script, above 24's `-- @version: 11.0+`; the header counted as a statement, the floor was lost, and PART H's `-- @version: any` re-enabled a section whose fixtures (WFTest.OrderContext, 11.0+) had been skipped", "file": "`mdl/executor/roundtrip_doctype_test.go` (filterByVersion)", "insight": "The language header is not a statement: `langver.IsHeaderLine` excludes it. The earlier regression test used a synthetic script without a header, so a corpus-wide header migration could not trip it — the new guard also runs filterByVersion on the real 24-workflow-examples.mdl. When a script-format migration lands, re-run the line-oriented tooling that reads those scripts (version gating, skip lists) against the real files, not synthetic ones. Repro: `MX_BINARY=~/.mxcli/mxbuild/10.24.24.119349/modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts/24-workflow ./mdl/executor/` (fails with exactly 481 skipped lines without the fix, 523 with it)", "refs": ["#901"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`create association M.X_Task from M.X to System.WorkflowUserTask` writes the association, then fails with `failed to reconcile access rules of module System: open …/00000000-0000-0000-0000-000000000002.mxunit: no such file`; the rest of the script is aborted. A re-run with `create or modify` is clean", "cause": "`reconcileModuleAccess` reconciles the TO end's module for a cross-module association; for System that is the virtual domain model, which has no stored unit to load. The `create or modify` re-run returns before reconciling, which hid it", "file": "`mdl/executor/cmd_associations.go` (`reconcileModuleAccess`)", "insight": "System is a module with a domain model but no unit — any per-module write sweep must skip it (`isBuiltinModuleEntity`). The alter-owner path reaches the same function", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`exec` prints `Created workflow: M.X` on every re-run of a `create or modify workflow`, even when `mxcli diff` says nothing would be written", "cause": "`execCreateWorkflow` printed a fixed `Created workflow:` line for both the create and the in-place update branch, bypassing `ReportMutation`, so write elision never surfaced as `Unchanged`", "file": "`mdl/executor/cmd_workflows_write.go` (`execCreateWorkflow`)", "insight": "Every mutating handler must report through `ctx.ReportMutation` (Created/Modified, downgraded to Unchanged on elision); a hand-rolled Fprintf is what makes a no-op look like churn", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "combo box `CaptionAttribute: FullName` on an entity extending Administration.Account fails mxbuild with CE1613 (`BIA.Employee.FullName` no longer exists); `Administration.Account.FullName` works. check and lint pass", "cause": "the widget engine's CaptionAttribute mapping qualified the name by concatenating the context entity, while every other attribute mapping goes through `resolveAttributePathForEntity`, which finds the declaring entity (the #12 inheritance fix missed this case)", "file": "`mdl/executor/widget_engine.go` (`resolveMapping`, case CaptionAttribute)", "insight": "Grep for `entity + \".\" +` when a CE1613 on an inherited attribute is reported: each hand-qualified path is a separate instance of the same defect", "refs": []} diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index c49c84683c..b5755d1dc8 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -1432,8 +1432,11 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W case "CaptionAttribute": if captionAttr := w.GetStringProp("CaptionAttribute"); captionAttr != "" { + // Qualified with the entity that DECLARES the attribute, as every + // other attribute mapping is: an inherited caption qualified with + // the specialization is CE1613 at build time. if entity := e.entityContextFor(mapping.PropertyKey); !strings.Contains(captionAttr, ".") && entity != "" { - captionAttr = entity + "." + captionAttr + captionAttr = e.pageBuilder.resolveAttributePathForEntity(captionAttr, entity) } ctx.AttributePath = captionAttr } diff --git a/mdl/executor/widget_engine_caption_inherited_test.go b/mdl/executor/widget_engine_caption_inherited_test.go new file mode 100644 index 0000000000..b8687e61aa --- /dev/null +++ b/mdl/executor/widget_engine_caption_inherited_test.go @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// A combo box's `CaptionAttribute: FullName` over an entity that extends +// Administration.Account was stored as `.FullName` and mxbuild failed +// with CE1613 "The selected attribute … no longer exists". Every other +// attribute mapping in the engine qualifies through resolveAttributePathForEntity, +// which names the DECLARING entity; the caption concatenated the context entity. +func TestResolveMapping_CaptionAttribute_Inherited(t *testing.T) { + for _, tc := range []struct { + attr, want string + }{ + {"FullName", "Administration.Account.FullName"}, // inherited + {"IsAvailable", "TaskBoard.Person.IsAvailable"}, // own (control) + {"Administration.Account.Email", "Administration.Account.Email"}, // already qualified + } { + t.Run(tc.attr, func(t *testing.T) { + engine := &PluggableWidgetEngine{pageBuilder: inheritancePB("TaskBoard.Person")} + mapping := PropertyMapping{PropertyKey: "optionsSourceAssociationCaptionAttribute", Source: "CaptionAttribute"} + w := &ast.WidgetV3{Properties: map[string]any{"CaptionAttribute": tc.attr}} + ctx, err := engine.resolveMapping(mapping, w) + if err != nil { + t.Fatalf("resolveMapping: %v", err) + } + if ctx.AttributePath != tc.want { + t.Errorf("caption attribute path %q, want %q", ctx.AttributePath, tc.want) + } + }) + } +} From 38d54574c6fd71389f1f200b684f6f9fb74bd8f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:06:53 +0000 Subject: [PATCH 07/60] fix(describe): a combo box with OnChange keeps its Attribute and caption OnChange routed a combo box into the generic pluggable branch, which emits no Attribute: or CaptionAttribute:, so describe -> exec dropped the binding. It now counts only for widgets without a dedicated describe branch. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + ...d_pages_describe_combobox_onchange_test.go | 53 +++++++++++++++++++ mdl/executor/cmd_pages_describe_output.go | 8 ++- 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 mdl/executor/cmd_pages_describe_combobox_onchange_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 37f72b8d6d..487e7a55a2 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -803,3 +803,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`create association M.X_Task from M.X to System.WorkflowUserTask` writes the association, then fails with `failed to reconcile access rules of module System: open …/00000000-0000-0000-0000-000000000002.mxunit: no such file`; the rest of the script is aborted. A re-run with `create or modify` is clean", "cause": "`reconcileModuleAccess` reconciles the TO end's module for a cross-module association; for System that is the virtual domain model, which has no stored unit to load. The `create or modify` re-run returns before reconciling, which hid it", "file": "`mdl/executor/cmd_associations.go` (`reconcileModuleAccess`)", "insight": "System is a module with a domain model but no unit — any per-module write sweep must skip it (`isBuiltinModuleEntity`). The alter-owner path reaches the same function", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`exec` prints `Created workflow: M.X` on every re-run of a `create or modify workflow`, even when `mxcli diff` says nothing would be written", "cause": "`execCreateWorkflow` printed a fixed `Created workflow:` line for both the create and the in-place update branch, bypassing `ReportMutation`, so write elision never surfaced as `Unchanged`", "file": "`mdl/executor/cmd_workflows_write.go` (`execCreateWorkflow`)", "insight": "Every mutating handler must report through `ctx.ReportMutation` (Created/Modified, downgraded to Unchanged on elision); a hand-rolled Fprintf is what makes a no-op look like churn", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "combo box `CaptionAttribute: FullName` on an entity extending Administration.Account fails mxbuild with CE1613 (`BIA.Employee.FullName` no longer exists); `Administration.Account.FullName` works. check and lint pass", "cause": "the widget engine's CaptionAttribute mapping qualified the name by concatenating the context entity, while every other attribute mapping goes through `resolveAttributePathForEntity`, which finds the declaring entity (the #12 inheritance fix missed this case)", "file": "`mdl/executor/widget_engine.go` (`resolveMapping`, case CaptionAttribute)", "insight": "Grep for `entity + \".\" +` when a CE1613 on an inherited attribute is reported: each hand-qualified path is a separate instance of the same defect", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`describe page` prints a combo box with `OnChange:` but without its `Attribute:` / `CaptionAttribute:` (only Label/DataSource/OnChange); the stored binding is correct and works at runtime, but describe → exec loses it", "cause": "`w.OnChange != \"\"` was added to the generic pluggable branch's has-content test for Slider/StarRating; that branch precedes the combo box's own branch and does not emit `Content` or `CaptionAttribute`", "file": "`mdl/executor/cmd_pages_describe_output.go` (`outputWidgetMDLV3`, generic pluggable branch)", "insight": "Widening the generic branch's guard captures known widgets with dedicated branches; gate the new term with `!isKnownCustomWidgetType`. A describe test per known widget with each newly-counted property would have caught it", "refs": []} diff --git a/mdl/executor/cmd_pages_describe_combobox_onchange_test.go b/mdl/executor/cmd_pages_describe_combobox_onchange_test.go new file mode 100644 index 0000000000..6cc69e12d3 --- /dev/null +++ b/mdl/executor/cmd_pages_describe_combobox_onchange_test.go @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" +) + +// A combo box with an `OnChange:` described back without its `Attribute:` (and, +// in association mode, without `CaptionAttribute:`): OnChange routed it into the +// generic pluggable branch meant for Slider/StarRating, which knows nothing of a +// combo box's binding. The stored widget was correct; describe → exec lost it. +func TestDescribeComboboxWithOnChangeKeepsItsBinding(t *testing.T) { + for _, tc := range []struct { + name string + w rawWidget + want []string + }{ + { + name: "enumeration mode", + w: rawWidget{ + Type: "CustomWidgets$CustomWidget", RenderMode: "combobox", Name: "cbColor", + WidgetID: "com.mendix.widget.web.combobox.Combobox", + Caption: "Color", Content: "Color", OnChange: "microflow M.OnColor", + }, + want: []string{"combobox cbColor", "Attribute: Color", "OnChange: microflow M.OnColor"}, + }, + { + name: "association mode", + w: rawWidget{ + Type: "CustomWidgets$CustomWidget", RenderMode: "combobox", Name: "cbEmp", + WidgetID: "com.mendix.widget.web.combobox.Combobox", + Content: "M.Task_Employee", CaptionAttribute: "Name", + DataSource: &rawDataSource{Type: "database", Reference: "M.Employee"}, + OnChange: "microflow M.OnEmp", + }, + want: []string{"combobox cbEmp", "Attribute: M.Task_Employee", "CaptionAttribute: Name", "OnChange: microflow M.OnEmp"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + outputWidgetMDLV3(&ExecContext{Output: &buf}, tc.w, 0) + out := buf.String() + for _, want := range tc.want { + if !strings.Contains(out, want) { + t.Errorf("description lacks %q:\n%s", want, out) + } + } + }) + } +} diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 058d522379..1cb2456135 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -779,13 +779,17 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { props = appendAppearanceProps(ctx, props, w) formatWidgetProps(ctx.Output, prefix, header, props, "\n") } else if (len(w.ExplicitProperties) > 0 || len(w.ObjectLists) > 0 || w.OnClick != "" || - w.OnChange != "" || len(w.NamedActions) > 0 || len(w.ChildSlots) > 0 || - len(w.OmittedContainers) > 0) && w.WidgetID != "" { + (w.OnChange != "" && !isKnownCustomWidgetType(widgetType)) || len(w.NamedActions) > 0 || + len(w.ChildSlots) > 0 || len(w.OmittedContainers) > 0) && w.WidgetID != "" { // Generic pluggable widget with explicit properties, object-list child // blocks (chart series/lines/scaleColors), and/or an onClick action. // The widget's own MDL name where that round-trips, else the // explicit id form. See pluggableWidgetHeader. // + // OnChange counts only for a widget without its own branch below: a + // combo box's branch emits OnChange itself, and routed here it lost + // its Attribute: and CaptionAttribute: on describe. + // // Child slots and omitted containers count towards "has content" // too: a widget whose only non-default content is a populated slot // took the bare branch below, which emits a head and no body — so From 8052f6167c0174b198832e29feced402ec4de6f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:07:38 +0000 Subject: [PATCH 08/60] fix(theme): dropping a vendored font family keeps the partial's braces balanced The @font-face pattern stopped at the `}` of the `#{$weight}` interpolation, leaving each dropped @each block's closing brace behind, so a seeded theme on any shipped base (console: 27 { vs 29 }) did not compile. The block's end is now found by counting braces. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + cmd/mxcli/theme/create_seeded.go | 55 ++++++++++++++++++- cmd/mxcli/theme/create_seeded_test.go | 39 +++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index fd83e3c7dc..0e85d2b4a0 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -141,3 +141,4 @@ {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli lsp --stdio` exits rc=2 with `panic: only file URIs are supported, got mendix-mdl` on textDocument/didOpen of a `mendix-mdl:` virtual document (the VS Code extension's describe previews); VS Code restarts it, it crashes again, and after 5 crashes it stops restarting the server", "cause": "checkableDocument (diagnostics) and CodeAction called go.lsp.dev/uri URI.Filename(), which panics on any scheme but file, to decide whether the document is a .test.mdl", "fix": "documentPath(uri) returns Filename() only for file: URIs and the URI's path component otherwise; runSemanticCheck (which shells out `mxcli check `) skips non-file documents; virtual documents are still diagnosed in memory", "insight": "Third-party helpers that panic on unexpected input are a crash path in a long-running server; every URI an LSP client sends is untrusted shape. A test with a non-file URI plus a file-URI control (same diagnostics) proves the virtual case is handled, not skipped", "issue": "mendixlabs/mxcli#1245", "file": "cmd/mxcli/lsp_helpers.go (documentPath, isFileURI); cmd/mxcli/lsp_diagnostics.go; cmd/mxcli/lsp_language.go", "test": "cmd/mxcli/lsp_virtual_uri_test.go"} {"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`mxcli test --local` fails to chain the project's after-startup microflow, then cleanup restores `AfterStartupMicroflow: 'AfterStartupMicroflow: ''Mod.Flow'` and the project is left pointing at `MxTest.RegisterEndpoint`", "cause": "`parseSettingValue` scraped DESCRIBE SETTINGS text and split only on `=`; the describe rewrite changed the output to `Key: 'value'`, so the whole line was kept as the value. The #803 tests used the old `=` lines only and stayed green against output describe no longer emits", "file": "`cmd/mxcli/testrunner/runner.go` (`parseSettingValue`, `getAfterStartup`)", "insight": "A parser of another command's human-readable output breaks silently when that output changes; the table test must carry the line the producer emits *today*. Split on the first `:` or `=`, unescape `''`, and match the key as a line prefix. Longer term, read the setting through the backend instead of scraping describe", "refs": []} {"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`mxcli check tests/X.test.mdl` (and the VS Code LSP) warns MDL-V1-SLASH once per test, on the doc-comment line, although the skill says a test file takes no `mdl 1;` header", "cause": "`CheckSource` renders each block as a microflow and closed the wrapper with `END; /`; the rendering is parsed headerless, so the visitor recorded a V1 slash note on a `/` mxcli itself wrote. The author's `/` separators are blanked and never reach the parser", "file": "`cmd/mxcli/testrunner/check_source.go`", "insight": "Generated MDL must be canonical under every language version, or the diagnostics blame the author's file. `TestCheckSourceWrapperIsCanonical` asserted no Deprecations but not LanguageNotes; it now asserts both", "refs": []} +{"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`theme create --from --base console` writes `_mxcli-.scss` with a stray `}}`/`}` where the @font-face section was (27 `{` vs 29 `}`); the theme does not compile. Same for --base signal and ledger", "cause": "`dropFontFaces` matched each `@each $weight { @font-face { … } }` block with `[^}]*\\}[^}]*\\}`; the `src: url(\"…-#{$weight}-…\")` interpolation's `}` ended the first span early, so the match stopped at the @font-face close and left the @each's `}`", "file": "`cmd/mxcli/theme/create_seeded.go` (`dropFontFaces`, now `dropFontFaceBlock`)", "insight": "Never match nested SCSS blocks with `[^}]*`; count braces (interpolations are balanced). The existing test used the real shape but asserted only absence of @font-face — any rewrite of a stylesheet should assert brace balance on every shipped asset. Not copying the fonts folder is intended when the design's font stacks name none of the vendored families", "refs": []} diff --git a/cmd/mxcli/theme/create_seeded.go b/cmd/mxcli/theme/create_seeded.go index a32ef7bac7..c21d92c71b 100644 --- a/cmd/mxcli/theme/create_seeded.go +++ b/cmd/mxcli/theme/create_seeded.go @@ -133,9 +133,7 @@ func unusedVendoredFamilies(scss string, tokens *Tokens) []string { // section comment when nothing is left to explain. func dropFontFaces(scss string, families []string) string { for _, fam := range families { - re := regexp.MustCompile(`(?s)\n*@each\s+\$weight[^{]*\{\s*@font-face\s*\{[^}]*font-family:\s*"` + - regexp.QuoteMeta(fam) + `"[^}]*\}[^}]*\}`) - scss = re.ReplaceAllString(scss, "") + scss = dropFontFaceBlock(scss, fam) } if len(vendoredFamilies(scss)) == 0 { // The banner explains vendored fonts; with none left it describes @@ -146,6 +144,57 @@ func dropFontFaces(scss string, families []string) string { return strings.TrimRight(scss, "\n") + "\n" } +// eachWeightRe finds the start of a per-family `@each $weight` block, with the +// blank lines before it. +var eachWeightRe = regexp.MustCompile(`\n*@each\s+\$weight[^{]*\{`) + +// dropFontFaceBlock removes every `@each $weight` block that declares family. +// +// The block's end is found by counting braces, not by a pattern: its `src:` +// interpolates `#{$weight}`, whose `}` a `[^}]*\}` pattern took for the +// @font-face close — so the @each's own `}` was left behind and the partial no +// longer compiled. An interpolation's braces are balanced, so depth counting +// steps over them. +func dropFontFaceBlock(scss, family string) string { + familyRe := regexp.MustCompile(`font-family:\s*"` + regexp.QuoteMeta(family) + `"`) + var out strings.Builder + rest := scss + for { + loc := eachWeightRe.FindStringIndex(rest) + if loc == nil { + break + } + end := closingBrace(rest, loc[1]-1) + if end < 0 { + break // unbalanced: leave the rest as written rather than guess + } + out.WriteString(rest[:loc[0]]) + if !familyRe.MatchString(rest[loc[1]:end]) { + out.WriteString(rest[loc[0] : end+1]) + } + rest = rest[end+1:] + } + out.WriteString(rest) + return out.String() +} + +// closingBrace returns the index of the `}` matching the `{` at open, or -1. +func closingBrace(s string, open int) int { + depth := 0 + for i := open; i < len(s); i++ { + switch s[i] { + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return i + } + } + } + return -1 +} + // fontSectionCommentRe matches the vendored-fonts banner comment. var fontSectionCommentRe = regexp.MustCompile(`(?m)\n*^// -+\n(?:^// .*\n)*?^// Fonts\. Vendored.*\n(?:^//.*\n)*^// -+\n`) diff --git a/cmd/mxcli/theme/create_seeded_test.go b/cmd/mxcli/theme/create_seeded_test.go index 3eb8ca89e5..9e45913258 100644 --- a/cmd/mxcli/theme/create_seeded_test.go +++ b/cmd/mxcli/theme/create_seeded_test.go @@ -274,3 +274,42 @@ func TestScaffoldedThemeShipsExactlyTheFontsItLoads(t *testing.T) { }) } } + +// Dropping a family left its `@each` block's closing brace behind: the pattern +// stopped at the first `}` after the family name, which is the one closing the +// `#{$weight}` interpolation in `src:`, so the `@font-face` close was taken for +// the `@each` close. `theme create --from … --base console` wrote a partial with +// 27 `{` and 29 `}`. Every shipped base has the same shape; check each. +func TestDropFontFacesKeepsBracesBalanced(t *testing.T) { + balanced := func(t *testing.T, label, s string) { + t.Helper() + if o, c := strings.Count(s, "{"), strings.Count(s, "}"); o != c { + t.Errorf("%s: %d `{` against %d `}`", label, o, c) + } + } + balanced(t, "probe, one family", dropFontFaces(probePartial, []string{"IBM Plex Sans"})) + balanced(t, "probe, all families", dropFontFaces(probePartial, []string{"IBM Plex Sans", "IBM Plex Mono"})) + + partials, _ := filepath.Glob("assets/*/files/theme/web/_mxcli-*.scss") + checked := 0 + for _, p := range partials { + body, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + fams := vendoredFamilies(string(body)) + if len(fams) == 0 { + continue + } + checked++ + balanced(t, p+" (as shipped)", string(body)) + got := dropFontFaces(string(body), fams) + balanced(t, p+" (all families dropped)", got) + if strings.Contains(got, "@font-face") { + t.Errorf("%s: @font-face survived dropping every family", p) + } + } + if checked == 0 { + t.Fatal("no shipped partial declares a vendored family — the glob no longer finds the assets") + } +} From 8d704af0f98d670fc4ed5181437546ecf3f289c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:10:59 +0000 Subject: [PATCH 09/60] fix(rename): renaming a view entity renames its OQL source document rename entity left the DomainModels$ViewEntitySourceDocument under the old name and persisted the entity's stale SourceDocument pointer, giving CE6784; recreating the view then orphaned the old document (CE6786). The rename now carries the pointer and renames the document, found by type and name. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + mdl/backend/domainmodel.go | 3 + mdl/backend/mcp/unsupported_gen.go | 5 ++ mdl/backend/mock/backend.go | 1 + mdl/backend/mock/mock_domainmodel.go | 7 ++ mdl/backend/modelsdk/move_view_write.go | 34 +++++++++ mdl/backend/modelsdk/unimplemented_gen.go | 4 + mdl/executor/cmd_rename.go | 18 +++++ mdl/executor/rename_view_entity_test.go | 76 +++++++++++++++++++ 9 files changed, 149 insertions(+) create mode 100644 mdl/executor/rename_view_entity_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 487e7a55a2..f13c998fd7 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -804,3 +804,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`exec` prints `Created workflow: M.X` on every re-run of a `create or modify workflow`, even when `mxcli diff` says nothing would be written", "cause": "`execCreateWorkflow` printed a fixed `Created workflow:` line for both the create and the in-place update branch, bypassing `ReportMutation`, so write elision never surfaced as `Unchanged`", "file": "`mdl/executor/cmd_workflows_write.go` (`execCreateWorkflow`)", "insight": "Every mutating handler must report through `ctx.ReportMutation` (Created/Modified, downgraded to Unchanged on elision); a hand-rolled Fprintf is what makes a no-op look like churn", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "combo box `CaptionAttribute: FullName` on an entity extending Administration.Account fails mxbuild with CE1613 (`BIA.Employee.FullName` no longer exists); `Administration.Account.FullName` works. check and lint pass", "cause": "the widget engine's CaptionAttribute mapping qualified the name by concatenating the context entity, while every other attribute mapping goes through `resolveAttributePathForEntity`, which finds the declaring entity (the #12 inheritance fix missed this case)", "file": "`mdl/executor/widget_engine.go` (`resolveMapping`, case CaptionAttribute)", "insight": "Grep for `entity + \".\" +` when a CE1613 on an inherited attribute is reported: each hand-qualified path is a separate instance of the same defect", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`describe page` prints a combo box with `OnChange:` but without its `Attribute:` / `CaptionAttribute:` (only Label/DataSource/OnChange); the stored binding is correct and works at runtime, but describe → exec loses it", "cause": "`w.OnChange != \"\"` was added to the generic pluggable branch's has-content test for Slider/StarRating; that branch precedes the combo box's own branch and does not emit `Content` or `CaptionAttribute`", "file": "`mdl/executor/cmd_pages_describe_output.go` (`outputWidgetMDLV3`, generic pluggable branch)", "insight": "Widening the generic branch's guard captures known widgets with dedicated branches; gate the new term with `!isKnownCustomWidgetType`. A describe test per known widget with each newly-counted property would have caught it", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`rename entity M.VW_X to X` on a view entity: mxbuild CE6784 \"View Entity name is out of sync with the OQL query name\"; re-running `create or modify view entity M.X` then orphans the old source document (CE6786), which no MDL statement can drop", "cause": "`execRenameEntity` never renamed the `DomainModels$ViewEntitySourceDocument`, and `UpdateDomainModel` persisted the entity's stale `SourceDocumentRef`, undoing the RenameReferences sweep's rewrite (the same clobber `repointEntitySelfRefs` fixes for member names)", "file": "`mdl/executor/cmd_rename.go` (`execRenameEntity`), `mdl/backend/modelsdk/move_view_write.go` (`RenameViewEntitySourceDocument`)", "insight": "A view entity is two units; every entity-level operation (move, rename, drop) must carry the source document. Rename it by $Type+name, not `RenameDocumentByName`, which matches any unit of that name. Verified on testapp-views: describe → exec after the rename reports Unchanged", "refs": []} diff --git a/mdl/backend/domainmodel.go b/mdl/backend/domainmodel.go index c383ee642a..dab26fb4b6 100644 --- a/mdl/backend/domainmodel.go +++ b/mdl/backend/domainmodel.go @@ -60,6 +60,9 @@ type DomainModelBackend interface { FindViewEntitySourceDocumentID(moduleName, docName string) (model.ID, error) FindAllViewEntitySourceDocumentIDs(moduleName, docName string) ([]model.ID, error) MoveViewEntitySourceDocument(sourceModuleName string, targetModuleID model.ID, docName string) error + // RenameViewEntitySourceDocument renames the module's source document + // oldName to newName; no-op when there is none. + RenameViewEntitySourceDocument(moduleName, oldName, newName string) error UpdateOqlQueriesForMovedEntity(oldQualifiedName, newQualifiedName string) (int, error) UpdateEnumerationRefsInAllDomainModels(oldQualifiedName, newQualifiedName string) error } diff --git a/mdl/backend/mcp/unsupported_gen.go b/mdl/backend/mcp/unsupported_gen.go index 12bbb6f2ea..82231d7208 100644 --- a/mdl/backend/mcp/unsupported_gen.go +++ b/mdl/backend/mcp/unsupported_gen.go @@ -1114,6 +1114,11 @@ func (unsupportedBackend) RenameReferences(_ string, _ string, _ bool) (r0 []typ return } +func (unsupportedBackend) RenameViewEntitySourceDocument(_ string, _ string, _ string) (err0 error) { + err0 = errUnsupported("RenameViewEntitySourceDocument") + return +} + func (unsupportedBackend) RevokeEntityMemberAccess(_ model.ID, _ string, _ []string, _ types.EntityAccessRevocation) (r0 int, err1 error) { err1 = errUnsupported("RevokeEntityMemberAccess") return diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index 8038d63b22..880d10f0c5 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -80,6 +80,7 @@ type MockBackend struct { WriteViewEntitySourceDocumentFunc func(moduleID model.ID, moduleName, docName, oqlQuery, documentation string) (model.ID, error) DeleteViewEntitySourceDocumentFunc func(id model.ID) error DeleteViewEntitySourceDocumentByNameFunc func(moduleName, docName string) error + RenameViewEntitySourceDocumentFunc func(moduleName, oldName, newName string) error FindViewEntitySourceDocumentIDFunc func(moduleName, docName string) (model.ID, error) FindAllViewEntitySourceDocumentIDsFunc func(moduleName, docName string) ([]model.ID, error) MoveViewEntitySourceDocumentFunc func(sourceModuleName string, targetModuleID model.ID, docName string) error diff --git a/mdl/backend/mock/mock_domainmodel.go b/mdl/backend/mock/mock_domainmodel.go index 199df21bb8..6d10e71df4 100644 --- a/mdl/backend/mock/mock_domainmodel.go +++ b/mdl/backend/mock/mock_domainmodel.go @@ -154,6 +154,13 @@ func (m *MockBackend) DeleteViewEntitySourceDocumentByName(moduleName, docName s return nil } +func (m *MockBackend) RenameViewEntitySourceDocument(moduleName, oldName, newName string) error { + if m.RenameViewEntitySourceDocumentFunc != nil { + return m.RenameViewEntitySourceDocumentFunc(moduleName, oldName, newName) + } + return nil +} + func (m *MockBackend) FindViewEntitySourceDocumentID(moduleName, docName string) (model.ID, error) { if m.FindViewEntitySourceDocumentIDFunc != nil { return m.FindViewEntitySourceDocumentIDFunc(moduleName, docName) diff --git a/mdl/backend/modelsdk/move_view_write.go b/mdl/backend/modelsdk/move_view_write.go index 51265e03f8..41d9ce0540 100644 --- a/mdl/backend/modelsdk/move_view_write.go +++ b/mdl/backend/modelsdk/move_view_write.go @@ -13,6 +13,7 @@ import ( "github.com/mendixlabs/mxcli/modelsdk/meta" mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" "github.com/mendixlabs/mxcli/modelsdk/mprread" + "go.mongodb.org/mongo-driver/bson" ) // MoveEnumeration reparents an enumeration unit to its (already-updated) target @@ -221,6 +222,39 @@ func (b *Backend) MoveViewEntitySourceDocument(sourceModuleName string, targetMo return b.writer.MoveUnit(string(docID), string(targetModuleID)) } +// RenameViewEntitySourceDocument renames a view entity's OQL source document, +// found by $Type as well as name — RenameDocumentByName matches any unit with +// that name, and a microflow or page may share it. A view entity's document +// must carry the entity's name (CE6784 otherwise). +func (b *Backend) RenameViewEntitySourceDocument(moduleName, oldName, newName string) error { + if b.writer == nil { + return fmt.Errorf("RenameViewEntitySourceDocument: not connected for writing") + } + docID, err := b.FindViewEntitySourceDocumentID(moduleName, oldName) + if err != nil || docID == "" { + return err // nil docID → nothing to rename + } + raw, err := b.reader.GetRawUnitBytes(string(docID)) + if err != nil { + return fmt.Errorf("RenameViewEntitySourceDocument: read: %w", err) + } + var doc bson.D + if err := bson.Unmarshal(raw, &doc); err != nil { + return fmt.Errorf("RenameViewEntitySourceDocument: decode: %w", err) + } + for i, elem := range doc { + if elem.Key == "Name" { + doc[i].Value = newName + contents, err := bson.Marshal(doc) + if err != nil { + return fmt.Errorf("RenameViewEntitySourceDocument: marshal: %w", err) + } + return b.writer.UpdateRawUnit(string(docID), contents) + } + } + return fmt.Errorf("RenameViewEntitySourceDocument: %s.%s has no Name", moduleName, oldName) +} + // FindAllViewEntitySourceDocumentIDs returns every ViewEntitySourceDocument unit // named docName in the given module. func (b *Backend) FindAllViewEntitySourceDocumentIDs(moduleName, docName string) ([]model.ID, error) { diff --git a/mdl/backend/modelsdk/unimplemented_gen.go b/mdl/backend/modelsdk/unimplemented_gen.go index c750aa4bf1..3a07f923cb 100644 --- a/mdl/backend/modelsdk/unimplemented_gen.go +++ b/mdl/backend/modelsdk/unimplemented_gen.go @@ -1010,6 +1010,10 @@ func (unimplemented) RenameReferences(_ string, _ string, _ bool) ([]types.Renam return r0, errUnimplemented("RenameReferences") } +func (unimplemented) RenameViewEntitySourceDocument(_ string, _ string, _ string) error { + return errUnimplemented("RenameViewEntitySourceDocument") +} + func (unimplemented) RevokeEntityMemberAccess(_ model.ID, _ string, _ []string, _ types.EntityAccessRevocation) (int, error) { var r0 int return r0, errUnimplemented("RevokeEntityMemberAccess") diff --git a/mdl/executor/cmd_rename.go b/mdl/executor/cmd_rename.go index ed5f5261f3..4d50e819f9 100644 --- a/mdl/executor/cmd_rename.go +++ b/mdl/executor/cmd_rename.go @@ -90,16 +90,34 @@ func execRenameEntity(ctx *ExecContext, s *ast.RenameStmt) error { } // Update the entity name in the domain model + isView := false for _, ent := range dm.Entities { if ent.Name == s.Name.Name { ent.Name = s.NewName repointEntitySelfRefs(ent, oldQualifiedName, newQualifiedName) + // A view entity's OQL source document carries the entity's name, + // and the entity points at it by qualified name. The sweep above + // rewrites the pointer in the raw unit, but this persist would put + // the stale one back — the same clobber repointEntitySelfRefs undoes + // for member names — and the document itself kept the old name: + // CE6784 "View Entity name is out of sync with the OQL query name". + if ent.Source == "DomainModels$OqlViewEntitySource" { + isView = true + if ent.SourceDocumentRef == oldQualifiedName { + ent.SourceDocumentRef = newQualifiedName + } + } break } } if err := ctx.Backend.UpdateDomainModel(dm); err != nil { return mdlerrors.NewBackend("update entity name", err) } + if isView { + if err := ctx.Backend.RenameViewEntitySourceDocument(s.Name.Module, s.Name.Name, s.NewName); err != nil { + return mdlerrors.NewBackend("rename view entity source document", err) + } + } invalidateHierarchy(ctx) invalidateDomainModelsCache(ctx) diff --git a/mdl/executor/rename_view_entity_test.go b/mdl/executor/rename_view_entity_test.go new file mode 100644 index 0000000000..b04198fe4f --- /dev/null +++ b/mdl/executor/rename_view_entity_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// `rename entity BIA.VW_StatusTotals to StatusTotals` renamed the entity and its +// references but not its DomainModels$ViewEntitySourceDocument, and the persist +// put the entity's stale SourceDocument reference back over the sweep's +// rewrite: CE6784 "View Entity name is out of sync with the OQL query name". +// Recreating the view under the new name then orphaned the old document +// (CE6786), which no MDL statement can remove. +func TestRename_ViewEntity_RenamesItsSourceDocument(t *testing.T) { + for _, tc := range []struct { + name string + source string + wantRename bool + }{ + {"view entity", "DomainModels$OqlViewEntitySource", true}, + {"persistent entity (control)", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("BIA") + ent := mkEntity(mod.ID, "VW_StatusTotals") + ent.Source = tc.source + if tc.source != "" { + ent.SourceDocumentRef = "BIA.VW_StatusTotals" + } + dm := mkDomainModel(mod.ID, ent) + var renamed []string + var persisted string + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetDomainModelFunc: func(model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + RenameReferencesFunc: func(string, string, bool) ([]types.RenameHit, error) { + return nil, nil + }, + UpdateDomainModelFunc: func(d *domainmodel.DomainModel) error { + persisted = d.Entities[0].SourceDocumentRef + return nil + }, + RenameViewEntitySourceDocumentFunc: func(module, oldName, newName string) error { + renamed = []string{module, oldName, newName} + return nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + assertNoError(t, execRename(ctx, &ast.RenameStmt{ + ObjectType: "entity", + Name: ast.QualifiedName{Module: "BIA", Name: "VW_StatusTotals"}, + NewName: "StatusTotals", + })) + if !tc.wantRename { + if renamed != nil { + t.Fatalf("renamed a source document %v for an entity that has none", renamed) + } + return + } + if len(renamed) != 3 || renamed[0] != "BIA" || renamed[1] != "VW_StatusTotals" || renamed[2] != "StatusTotals" { + t.Fatalf("source document rename %v, want [BIA VW_StatusTotals StatusTotals]", renamed) + } + if persisted != "BIA.StatusTotals" { + t.Fatalf("persisted SourceDocument %q, want BIA.StatusTotals", persisted) + } + }) + } +} From c86b09adbac8be372d269ba95059c0ed07319248 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:11:03 +0000 Subject: [PATCH 10/60] style: gofmt reserved_keyword_hint_test.go Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- mdl/visitor/reserved_keyword_hint_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/mdl/visitor/reserved_keyword_hint_test.go b/mdl/visitor/reserved_keyword_hint_test.go index f9cb39c2f8..e26a474e0d 100644 --- a/mdl/visitor/reserved_keyword_hint_test.go +++ b/mdl/visitor/reserved_keyword_hint_test.go @@ -19,7 +19,6 @@ import ( // fix where it applies; the hint still serves the slots that do not. const keywordSlot = "create workflow M.W\nbegin\n user task %s 'Review'\n page M.P;\nend workflow;" - func hintFor(t *testing.T, name string) string { t.Helper() _, errs := Build(strings.Replace(keywordSlot, "%s", name, 1)) From 42011515b6d9b9aeca015613ce41cbbea6de6120 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:12:25 +0000 Subject: [PATCH 11/60] fix(lint): SEC008 counts only roles that can read a PII attribute The rule took the entity-level READ row, which the catalog emits when any member is readable, so a role granted read on FullName alone was reported as reading Email. It now matches unconstrained MEMBER_READ rows on the PII attributes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../sec_unconstrained_pii_read.star | 21 +++-- .../skills/fix-issue/findings/mdl-other.jsonl | 1 + mdl/linter/starlark_sec008_test.go | 89 +++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 mdl/linter/starlark_sec008_test.go diff --git a/.claude/lint-rules/sec_unconstrained_pii_read.star b/.claude/lint-rules/sec_unconstrained_pii_read.star index 36d1b8d1bb..3718236b34 100644 --- a/.claude/lint-rules/sec_unconstrained_pii_read.star +++ b/.claude/lint-rules/sec_unconstrained_pii_read.star @@ -52,18 +52,29 @@ def check(): if len(pii_attrs) == 0: continue - # Find roles with unconstrained READ + # Find roles that can read a PII attribute with no row constraint. The + # entity-level READ row is emitted when ANY member is readable, so it + # cannot answer this: a role granted `read (FullName)` has it too. The + # member row can. Its name is qualified for explicit member rights and + # bare when expanded from default rights, so match either spelling. unconstrained_roles = [] + readable_pii = [] for perm in permissions_for(e.qualified_name): - if perm.access_type == "READ" and perm.member_name == "" and not perm.is_constrained: - unconstrained_roles.append(perm.module_role_name) + if perm.access_type != "MEMBER_READ" or perm.is_constrained: + continue + for attr_name in pii_attrs: + if perm.member_name == attr_name or perm.member_name.endswith("." + attr_name): + if perm.module_role_name not in unconstrained_roles: + unconstrained_roles.append(perm.module_role_name) + if attr_name not in readable_pii: + readable_pii.append(attr_name) if len(unconstrained_roles) > 0: violations.append(violation( message="Entity '{}' contains PII attributes ({}) and is readable without XPath row constraints by: {}".format( e.qualified_name, - ", ".join(pii_attrs), - ", ".join(unconstrained_roles), + ", ".join(readable_pii), + ", ".join(sorted(unconstrained_roles)), ), location=location( module=e.module_name, diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index c7a8b9e387..71b67fc23f 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -79,3 +79,4 @@ {"date": "2026-10-01", "area": "mdl/scriptdiff", "symptom": "`mxcli diff` disagrees with exec: after exec has applied a script (a second exec writes nothing), diff still reports the entity modified — `Success: Boolean` against the stored `Success: Boolean default false`, `Body: String` against `String(unlimited)`, an `@Position` line, a re-laid-out flow as 'N statements added'; and it is silent where exec writes (pages, translations, a layout repointed — 'Not compared'), reports a plain `create` of an existing element as unchanged although exec stops on 'already exists' (#807), and gives 'Could not diff' for a flow that calls one the script creates first (#856). Rehearsal 3: phantom changes on 66 scripts, silent on 37 that wrote", "cause": "diff answered 'what would exec write?' a second way: it rendered each statement as MDL with its own per-statement converters and compared the text with the stored document's describe output, without running any statement. Each fix (#997 one flow renderer, #794 module name, #839 the splice verdict, the storage carry) synchronised one more spot of the second answer, and every spelling exec normalises but the converter did not (implicit defaults, positions) stayed a phantom", "file": "mdl/scriptdiff/scriptdiff.go, cmd/mxcli/cmd_diff.go, cmd/mxcli/exec_preflight.go", "fix": "Remove the second answer: diff copies the project to a scratch folder, runs the script there with exec's own code (the same pre-flight, ExecuteProgram / ContinueOnError, the script's header), and compares the copy with the project unit by unit (bytes plus the Unit-table container, so a move is seen); the changed units are rendered by DESCRIBE on both sides (a domain model per entity/association, module and project security per role/demo user). The statement-based differ (cmd_diff_mdl.go, cmd_diff_render.go, spliceVerdict) is deleted. The scratch run always uses the file engine, also under --mcp", "test": "mdl/scriptdiff/scriptdiff_test.go (PedApp: applied script diffs empty with a control that the first run adds the entity; plain create refused; earlier statements run; layout change shown; folder move reported); property_test.go -tags integration: over mdl-examples doctype scripts, diff's written-unit set equals exec's, twice; MXCLI_DIFF_LEGS runs the rehearsal legs", "insight": "A preview that predicts a side-effecting command should BE that command run somewhere harmless, not a model of it. Every normalisation exec applies (canonical defaults, builtAsStored, write elision, the splice verdict) has to be re-derived by a renderer-based differ, and the drift only shows on real projects. Copying a 56 MB project and executing costs well under a second; seven rounds of synchronising the renderer did not converge. Also: a write that changes no unit bytes (a move) lives in the .mpr Unit table, so the snapshot must include each unit's container", "refs": ["#907", "#807", "#856"]} {"date": "2026-10-01", "area": "mdl/scriptdiff", "symptom": "`mxcli diff` of a headerless (mdl 0) script that starts with `connect local ''` wrote the script's changes into the real project and reported `exec would write nothing`; a connect to another project (or one inside an `execute script`) wrote that project; `sql ` and `import from` would run against real databases. Also: with TMPDIR inside the project folder the scratch copy copied itself until 'file name too long'", "cause": "diff executes the script for real on a scratch copy, but exec follows a CONNECT (allowed under mdl 0 with a MDL-V1-SESSION warning, and inside nested scripts under any header) and SQL/IMPORT act on databases, so 'run it on a copy' only holds for statements whose effects stay in the connected project. The snapshot compared was the copy's, which saw no write, so the report said nothing changed", "file": "mdl/scriptdiff/scriptdiff.go (outsideGuard), mdl/executor/executor.go (SetStatementGuard), mdl/scriptdiff/copy.go", "fix": "Executor.SetStatementGuard: a hook every statement passes through in Execute, nested EXECUTE SCRIPT included. diff's guard redirects a connect to the -p project onto the copy, and refuses a connect elsewhere, a SQL query and an IMPORT with an error (not as exec's verdict). copyProject skips the folder the copy is created in", "test": "mdl/scriptdiff/outside_test.go: connect to the project diffs onto the copy and agrees with exec, project untouched; connect to another project and a nested one refused, other project untouched; SQL query / import refused; scratch folder inside the project", "insight": "Running the real command on a copy is only a dry run for effects that stay inside the copy. Enumerate every statement that reaches outside the connected project (connect, SQL, import, nested scripts) and guard them at the dispatch point, not per top-level statement, or a nested script walks around it", "refs": ["#907", "#913"]} {"area": "mdl/roundtrip", "date": "2026-10-02", "symptom": "Nightly, every Mendix version: `TestFlowVerdictAgreement_Corpus` fails `no probe was refused or rebuilt: the agreement was never tested on a refusal` / `0 probes refused or rebuilt`, while push-test CI passes", "cause": "The guard's refusals came only from TestApp (testdata/testapp, a submodule); PedApp has no flow with a loop. nightly.yml checked out without submodules, so the TestApp subtest skipped and the control had nothing to count", "file": "`mdl/roundtrip/flow_verdict_agreement_test.go`, `.github/workflows/nightly.yml`", "insight": "A test's control must not depend on an optional fixture that can skip: a fully scanned fixture with no loop now gets an mxcli-authored loop flow (made part of the fixture copy via `harness.addToFixture`, so a verdict's restore keeps it). And any workflow that runs the roundtrip suite needs `submodules: true` — push-test.yml had it, nightly.yml did not, so the gap showed only in the nightly. Repro: run the test in a worktree without `git submodule update`"} +{"area": "lint-rules", "date": "2026-10-02", "symptom": "SEC008 reports Engineer.Email as readable without row constraints by roles whose `show access` shows Email = None (they were granted `read (FullName)` only)", "cause": "`sec_unconstrained_pii_read.star` counted the entity-level READ permission row, which the catalog emits when ANY member is readable (`entityAccessFromMemberRights`), so it says nothing about the PII attribute", "file": "`cmd/mxcli/lint-rules/sec_unconstrained_pii_read.star` and `.claude/lint-rules/` copy", "insight": "For member-level questions use MEMBER_READ rows. Their member_name is qualified (`M.E.Attr`) for explicit member rights but bare when expanded from DefaultMemberAccessRights — match both. SEC008 also only fires after a full catalog build (permissions table)", "refs": []} diff --git a/mdl/linter/starlark_sec008_test.go b/mdl/linter/starlark_sec008_test.go new file mode 100644 index 0000000000..0623e50bcb --- /dev/null +++ b/mdl/linter/starlark_sec008_test.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter_test + +import ( + "database/sql" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/linter" + _ "modernc.org/sqlite" +) + +// SEC008 flagged Engineer.Email as readable by every role with entity-level +// READ, but the catalog emits that row when ANY member is readable. A role +// granted `read (FullName)` — Email = None in `show access` — was reported as +// reading PII. Only a role that can read a PII member counts. +func TestSEC008CountsOnlyRolesThatReadAPiiMember(t *testing.T) { + db, err := sql.Open("sqlite", ":memory:") + if err != nil { + t.Fatal(err) + } + for _, s := range []string{ + `CREATE TABLE modules (Id TEXT, Name TEXT, Source TEXT)`, + `INSERT INTO modules VALUES ('m1', 'FS', '')`, + `CREATE TABLE entities ( + Id TEXT, Name TEXT, QualifiedName TEXT, ModuleName TEXT, Folder TEXT, + EntityType TEXT, Description TEXT, Generalization TEXT, + AttributeCount INTEGER, AccessRuleCount INTEGER, ValidationRuleCount INTEGER, + HasEventHandlers INTEGER, IsExternal INTEGER, + HasCreatedDate INTEGER, HasChangedDate INTEGER, + HasOwner INTEGER, HasChangedBy INTEGER)`, + `INSERT INTO entities VALUES + ('e1','Engineer','FS.Engineer','FS','','PERSISTENT','','',2,4,0,0,0, 0,0,0,0)`, + `CREATE TABLE attributes (Id TEXT, Name TEXT, EntityId TEXT, EntityQualifiedName TEXT, + ModuleName TEXT, DataType TEXT, Length INTEGER, IsUnique INTEGER, IsRequired INTEGER, + DefaultValue TEXT, IsCalculated INTEGER, Description TEXT)`, + `INSERT INTO attributes VALUES + ('a1','FullName','e1','FS.Engineer','FS','String',200,0,0,'',0,''), + ('a2','Email','e1','FS.Engineer','FS','String',200,0,0,'',0,'')`, + `CREATE TABLE permissions (ModuleRoleName TEXT, ElementType TEXT, ElementName TEXT, + MemberName TEXT, AccessType TEXT, XPathConstraint TEXT, + DefaultMemberAccessRights TEXT, ModuleName TEXT)`, + // Coordinator reads Email (explicit, qualified member name), Admin reads + // it through default rights (bare member name), FieldEngineer reads only + // FullName, Requester reads Email but row-scoped. + `INSERT INTO permissions VALUES + ('FS.Coordinator','ENTITY','FS.Engineer',NULL,'READ','','None','FS'), + ('FS.Coordinator','ENTITY','FS.Engineer','FS.Engineer.FullName','MEMBER_READ','','None','FS'), + ('FS.Coordinator','ENTITY','FS.Engineer','FS.Engineer.Email','MEMBER_READ','','None','FS'), + ('FS.Admin','ENTITY','FS.Engineer',NULL,'READ','','ReadOnly','FS'), + ('FS.Admin','ENTITY','FS.Engineer','FullName','MEMBER_READ','','ReadOnly','FS'), + ('FS.Admin','ENTITY','FS.Engineer','Email','MEMBER_READ','','ReadOnly','FS'), + ('FS.FieldEngineer','ENTITY','FS.Engineer',NULL,'READ','','None','FS'), + ('FS.FieldEngineer','ENTITY','FS.Engineer','FS.Engineer.FullName','MEMBER_READ','','None','FS'), + ('FS.Requester','ENTITY','FS.Engineer',NULL,'READ','[id = ''[%CurrentUser%]'']','None','FS'), + ('FS.Requester','ENTITY','FS.Engineer','FS.Engineer.Email','MEMBER_READ','[id = ''[%CurrentUser%]'']','None','FS')`, + } { + if _, err := db.Exec(s); err != nil { + t.Fatalf("fixture %q: %v", s, err) + } + } + + for _, dir := range []string{".claude/lint-rules", "cmd/mxcli/lint-rules"} { + t.Run(dir, func(t *testing.T) { + r, err := linter.LoadStarlarkRule(filepath.Join("..", "..", dir, "sec_unconstrained_pii_read.star")) + if err != nil { + t.Fatal(err) + } + got := r.Check(linter.NewLintContextFromDB(catalog.WrapSqlDB(db))) + if len(got) != 1 { + t.Fatalf("got %d violations, want 1: %v", len(got), got) + } + msg := got[0].Message + for _, want := range []string{"FS.Coordinator", "FS.Admin"} { + if !strings.Contains(msg, want) { + t.Errorf("violation does not name %s, which reads Email unconstrained:\n%s", want, msg) + } + } + for _, not := range []string{"FS.FieldEngineer", "FS.Requester"} { + if strings.Contains(msg, not) { + t.Errorf("violation names %s, which cannot read Email unconstrained:\n%s", not, msg) + } + } + }) + } +} From 80878cfcc713aa779e4a57553230c5f626c7cabd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:13:56 +0000 Subject: [PATCH 12/60] fix(security): a revoke with nothing to revoke says Unchanged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "No access rules found matching …" read as a failed lookup on the idempotent re-run of a script; it now reads "Unchanged entity access: M.E (roles) — nothing to revoke". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + mdl/executor/access_rule_run.go | 4 +- mdl/executor/cmd_security_write.go | 12 ++++- mdl/executor/revoke_nothing_wording_test.go | 47 +++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 mdl/executor/revoke_nothing_wording_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index f13c998fd7..5ef7452796 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -805,3 +805,4 @@ {"area": "mdl/executor", "date": "2026-10-02", "symptom": "combo box `CaptionAttribute: FullName` on an entity extending Administration.Account fails mxbuild with CE1613 (`BIA.Employee.FullName` no longer exists); `Administration.Account.FullName` works. check and lint pass", "cause": "the widget engine's CaptionAttribute mapping qualified the name by concatenating the context entity, while every other attribute mapping goes through `resolveAttributePathForEntity`, which finds the declaring entity (the #12 inheritance fix missed this case)", "file": "`mdl/executor/widget_engine.go` (`resolveMapping`, case CaptionAttribute)", "insight": "Grep for `entity + \".\" +` when a CE1613 on an inherited attribute is reported: each hand-qualified path is a separate instance of the same defect", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`describe page` prints a combo box with `OnChange:` but without its `Attribute:` / `CaptionAttribute:` (only Label/DataSource/OnChange); the stored binding is correct and works at runtime, but describe → exec loses it", "cause": "`w.OnChange != \"\"` was added to the generic pluggable branch's has-content test for Slider/StarRating; that branch precedes the combo box's own branch and does not emit `Content` or `CaptionAttribute`", "file": "`mdl/executor/cmd_pages_describe_output.go` (`outputWidgetMDLV3`, generic pluggable branch)", "insight": "Widening the generic branch's guard captures known widgets with dedicated branches; gate the new term with `!isKnownCustomWidgetType`. A describe test per known widget with each newly-counted property would have caught it", "refs": []} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`rename entity M.VW_X to X` on a view entity: mxbuild CE6784 \"View Entity name is out of sync with the OQL query name\"; re-running `create or modify view entity M.X` then orphans the old source document (CE6786), which no MDL statement can drop", "cause": "`execRenameEntity` never renamed the `DomainModels$ViewEntitySourceDocument`, and `UpdateDomainModel` persisted the entity's stale `SourceDocumentRef`, undoing the RenameReferences sweep's rewrite (the same clobber `repointEntitySelfRefs` fixes for member names)", "file": "`mdl/executor/cmd_rename.go` (`execRenameEntity`), `mdl/backend/modelsdk/move_view_write.go` (`RenameViewEntitySourceDocument`)", "insight": "A view entity is two units; every entity-level operation (move, rename, drop) must carry the source document. Rename it by $Type+name, not `RenameDocumentByName`, which matches any unit of that name. Verified on testapp-views: describe → exec after the rename reports Unchanged", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`revoke` of a right that is already gone prints \"No access rules found matching M.Role on M.Entity\" — reads like an error on an idempotent re-run (exit 0)", "cause": "the no-op branches of `execRevokeEntityAccess` (partial and full) worded the outcome as a failed lookup instead of the Unchanged form other idempotent writes use", "file": "`mdl/executor/cmd_security_write.go` (`execRevokeEntityAccess`, `nothingToRevoke`)", "insight": "Wording only; an idempotent no-op should say Unchanged so it is not mistaken for a failure in script logs", "refs": []} diff --git a/mdl/executor/access_rule_run.go b/mdl/executor/access_rule_run.go index 4b6afe9c1a..8ba119f6ca 100644 --- a/mdl/executor/access_rule_run.go +++ b/mdl/executor/access_rule_run.go @@ -53,8 +53,8 @@ type heldReport struct { // drops the text instead (a "Reconciled N rules" line about a rewrite that // did not happen). unchanged string - // notice is printed whatever the run wrote: it reports no write ("No access - // rules found …"), so there is nothing to downgrade. + // notice is printed whatever the run wrote: it reports no write ("… + // nothing to revoke"), so there is nothing to downgrade. notice bool } diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index 96be332dd2..b9f0dbef36 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -632,6 +632,14 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } // execRevokeEntityAccess handles REVOKE roles ON Module.Entity [(rights...)]. +// nothingToRevoke is a revoke's report when the roles already lack the rights: +// the idempotent re-run of a script, worded as the state it found rather than +// as a lookup that failed ("No access rules found matching …"). +func nothingToRevoke(entity ast.QualifiedName, roleNames []string) string { + return fmt.Sprintf("Unchanged entity access: %s.%s (%s) — nothing to revoke\n", + entity.Module, entity.Name, strings.Join(roleNames, ", ")) +} + func execRevokeEntityAccess(ctx *ExecContext, s *ast.RevokeEntityAccessStmt) error { if !ctx.ConnectedForWrite() { return mdlerrors.NewNotConnectedWrite() @@ -701,7 +709,7 @@ func execRevokeEntityAccess(ctx *ExecContext, s *ast.RevokeEntityAccessStmt) err } if modified == 0 { - ctx.reportAccessRule(heldReport{notice: true, text: fmt.Sprintf("No access rules found matching %s on %s.%s\n", strings.Join(roleNames, ", "), s.Entity.Module, s.Entity.Name)}) + ctx.reportAccessRule(heldReport{notice: true, text: nothingToRevoke(s.Entity, roleNames)}) } else { text := fmt.Sprintf("Revoked partial access on %s.%s from %s\n", s.Entity.Module, s.Entity.Name, strings.Join(roleNames, ", ")) if !ctx.Quiet { @@ -718,7 +726,7 @@ func execRevokeEntityAccess(ctx *ExecContext, s *ast.RevokeEntityAccessStmt) err } if modified == 0 { - ctx.reportAccessRule(heldReport{notice: true, text: fmt.Sprintf("No access rules found matching %s on %s.%s\n", strings.Join(roleNames, ", "), s.Entity.Module, s.Entity.Name)}) + ctx.reportAccessRule(heldReport{notice: true, text: nothingToRevoke(s.Entity, roleNames)}) } else { text := fmt.Sprintf("Revoked access on %s.%s from %s\n", s.Entity.Module, s.Entity.Name, strings.Join(roleNames, ", ")) if !ctx.Quiet { diff --git a/mdl/executor/revoke_nothing_wording_test.go b/mdl/executor/revoke_nothing_wording_test.go new file mode 100644 index 0000000000..ae2083fb26 --- /dev/null +++ b/mdl/executor/revoke_nothing_wording_test.go @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/security" +) + +// A revoke whose right is already gone is the idempotent re-run of a script, and +// said "No access rules found matching …" — which reads as a failed lookup. It +// is the revoke's "already in the state you asked for", worded like the grant's. +func TestRevokeWithNothingToRevokeSaysUnchanged(t *testing.T) { + mod := mkModule("MyModule") + dm := &domainmodel.DomainModel{ + BaseElement: model.BaseElement{ID: nextID("dm")}, + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{{BaseElement: model.BaseElement{ID: nextID("ent")}, Name: "Customer", Persistable: true}}, + } + ms := &security.ModuleSecurity{ContainerID: mod.ID, ModuleRoles: []*security.ModuleRole{{Name: "User"}}} + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetDomainModelFunc: func(model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + GetModuleSecurityFunc: func(model.ID) (*security.ModuleSecurity, error) { return ms, nil }, + ListModuleSecurityFunc: func() ([]*security.ModuleSecurity, error) { return []*security.ModuleSecurity{ms}, nil }, + RemoveEntityAccessRuleFunc: func(model.ID, string, []string) (int, error) { return 0, nil }, + } + ctx, out := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + assertNoError(t, execRevokeEntityAccess(ctx, &ast.RevokeEntityAccessStmt{ + Entity: ast.QualifiedName{Module: "MyModule", Name: "Customer"}, + Roles: []ast.QualifiedName{{Module: "MyModule", Name: "User"}}, + })) + got := out.String() + if strings.Contains(got, "No access rules found") { + t.Errorf("reads as a failed lookup: %q", got) + } + if !strings.Contains(got, "Unchanged entity access: MyModule.Customer (MyModule.User)") { + t.Errorf("output %q, want the Unchanged form", got) + } +} From 1db691845c0676ff800472e549b3c66bdad9b7fe Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:23:02 +0000 Subject: [PATCH 13/60] test(conformance): shrink the allowlist after keyword grant members parse Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- mdl/conformance/allowlist.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mdl/conformance/allowlist.txt b/mdl/conformance/allowlist.txt index d58446c11e..2a980c1598 100644 --- a/mdl/conformance/allowlist.txt +++ b/mdl/conformance/allowlist.txt @@ -93,7 +93,7 @@ 6 syntax docs-site/src/language/document-access.md 1 syntax docs-site/src/language/domain-model.md 2 syntax docs-site/src/language/entities.md -4 syntax docs-site/src/language/entity-access.md +3 syntax docs-site/src/language/entity-access.md 3 syntax docs-site/src/language/enumerations.md 2 syntax docs-site/src/language/event-services.md 7 syntax docs-site/src/language/expressions.md From 08968398a28875007622235698e9319feb22b6af Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:53:17 +0000 Subject: [PATCH 14/60] fix(check): give the contradictory-guard error its own id, MDL085 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `create or modify … if not exists` was reported as MDL067, the id the bare-commit INFO note (a bare `commit $X;` now runs events) also uses. One id for an error and an unrelated note made it useless for filtering or looking either one up. The guard error moves to MDL085, a gap no branch has ever used; the commit note keeps MDL067, which fmt --upgrade, mdl/upgrade and the released CHANGELOG already name for it. Docs, skills, the syntax help and the quick reference follow. The guard test now also asserts the old id is never reported for it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../generate-domain-model/reference/syntax.md | 2 +- CHANGELOG.md | 1 + cmd/mxcli/syntax/features_misc.go | 2 +- docs-site/src/language/basics.md | 2 +- docs/01-project/MDL_QUICK_REFERENCE.md | 2 +- mdl/ast/ast_create_guard.go | 2 +- mdl/executor/cmd_create_guard.go | 2 +- mdl/executor/cmd_enumerations.go | 4 ++-- .../validate_idempotency_guard_test.go | 23 ++++++++++++------- mdl/executor/validate_program.go | 2 +- .../visitor_create_if_not_exists_test.go | 2 +- 12 files changed, 27 insertions(+), 18 deletions(-) diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index b93a41eb07..3cc8a08a09 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -800,3 +800,4 @@ {"date": "2026-10-01", "area": "mdl/executor", "symptom": "mendixlabs/mxcli#175: `call workflow … on error continue` passed check and exec, then mx check: CE6035 at Call workflow activity. On 11.14.0 only continue fails: no clause, rollback and both custom handlers build.", "cause": "continueUnsupportedOn did not list call workflow, and adapters.StatementErrorHandling — a hand-kept list of statements carrying a clause — did not list CallWorkflowStmt (nor the REST/other workflow statements), so MDL076 could not even see the clause. MDL076 was also check-only: exec without the pre-check (-c, REPL, --no-check) wrote it.", "file": "mdl/executor/validate_microflow_error_handling.go; mdl/exprcheck/adapters/adapter_scope.go; mdl/executor/validate.go", "fix": "call workflow in continueUnsupportedOn; StatementErrorHandling falls back to reading any statement's ErrorHandling field by reflection (getErrorHandling delegates to it); MDL076 is exec-enforced.", "test": "TestMDL076_ReportsContinueOnCallWorkflow, TestMDL076_CallWorkflowAcceptsTheOtherClauses, TestMDL076_IsExecEnforced", "insight": "Three lists of 'statements with an ON ERROR clause' existed; each had drifted. A rule keyed on such a list silently does nothing for a statement the list forgot."} {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#571 / mendixlabs/mxcli#1206: `create published rest service` wrote only the path's {name} placeholders as operation parameters, each a String: every query and body microflow parameter failed mx check with CE0350, an Integer {id} with CE6539. `import mapping` / `export mapping` / `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on an object-returning microflow). Executing describe of a Studio Pro service (TestApp Services.OrdersRestApi) reported 'Modified' and broke a 0-error app with 5 errors: mappings cleared, Integer path params retyped String, body param dropped, Commit No->Yes, Basic+Session authentication turned off", "cause": "publishedRestOperationToGen built parameters from the path alone and wrote ExportMapping/ImportMapping \"\" and Commit \"Yes\" as constants; the reader never read parameters, mappings or commit, so describe could not print them and ALTER (which rewrites every operation) lost them too; the service writer also emitted constants for AuthenticationTypes / AuthenticationMicroflow / CorsConfiguration / Documentation / PublicDocumentation with no carry; Resources and operation Parameters were registered with list marker 2 where Studio Pro writes 3", "file": "mdl/executor/cmd_published_rest.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/export_level_carry.go, mdl/visitor/visitor_rest.go, model/types.go", "fix": "the executor derives operation parameters from the microflow as Studio Pro does (path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, else Query; the microflow parameter's type), merged over the stored parameters per bound microflow parameter so a header/renamed/described parameter survives; mappings and commit flow AST -> model -> BSON and back, describe prints them (commit when not Yes) and notes parameters MDL cannot state; an unknown commit value is refused at exec and by check (MDL-REST03); create or modify carries summary/documentation/object handling of the restated operation; UpdatePublishedRestService carries the stored service-level keys MDL cannot state (keepStoredTopLevel); list markers measured from TestApp", "test": "mdl/executor/cmd_published_rest_params_test.go; mdl/backend/modelsdk/published_rest_write_test.go TestCreatePublishedRestService_WritesParametersAndBindings, TestWithStoredTopLevel; mdl/roundtrip TestTestAppRoundTrip/published_rest_service_Services.OrdersRestApi (allowlist entry struck); mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl (TestApp copy: old binary 16 mx check errors, fixed 0, describe->exec Unchanged twice)", "insight": "A clause that parses and is then ignored is worse than a parse error: the grammar advertised import/export mapping for months while the writer hard-coded them empty. The fastest witness was the round-trip harness's own allowlist entry for the one Studio Pro published REST service in TestApp: removing it printed the whole loss set (bindings, parameter types, markers, authentication) in one diff. Studio Pro's metamodel (ped_get_schema over the MCP tunnel) gave the enum values and defaults: Commit defaults to No there, while mxcli keeps writing Yes when the clause is absent so existing scripts do not churn, and describe prints commit whenever it is not Yes."} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "Nightly, Mendix 10.24 only: `TestMxCheck_DoctypeScripts/24-workflow-examples.mdl/modelsdk` → `skipped 481 version-gated lines` then `Execution error: entity 'WFTest.OrderContext' not found for parameter 'OrderContext'` — the same failure TestFilterByVersion_FileBaselineSurvivesAny was written for, back again with that test green", "cause": "`filterByVersion` treats a `-- @version:` directive as the file's floor only if no statement precedes it. #901 put `mdl 1;` on line 1 of every doctype script, above 24's `-- @version: 11.0+`; the header counted as a statement, the floor was lost, and PART H's `-- @version: any` re-enabled a section whose fixtures (WFTest.OrderContext, 11.0+) had been skipped", "file": "`mdl/executor/roundtrip_doctype_test.go` (filterByVersion)", "insight": "The language header is not a statement: `langver.IsHeaderLine` excludes it. The earlier regression test used a synthetic script without a header, so a corpus-wide header migration could not trip it — the new guard also runs filterByVersion on the real 24-workflow-examples.mdl. When a script-format migration lands, re-run the line-oriented tooling that reads those scripts (version gating, skip lists) against the real files, not synthetic ones. Repro: `MX_BINARY=~/.mxcli/mxbuild/10.24.24.119349/modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts/24-workflow ./mdl/executor/` (fails with exactly 481 skipped lines without the fix, 523 with it)", "refs": ["#901"]} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "`MDL067` names two unrelated diagnostics: the ERROR for `create or modify … if not exists` (contradictory guards) and the INFO note that a bare `commit $X;` now runs events. A user filtering or looking up MDL067 cannot tell which one they have", "cause": "Rule ids are string literals at each `addViolation` / `RuleID:` site with no registry, so a later rule picked an id already in use and nothing failed", "file": "`mdl/executor/cmd_enumerations.go` (validateIdempotencyGuard), `mdl/executor/cmd_create_guard.go`, `mdl/executor/validate_commit_events.go`", "insight": "The guard error moved to MDL085 (a gap no branch's history ever used); the commit note kept MDL067 because `fmt --upgrade`, mdl/upgrade and the released CHANGELOG already name it for that note. Before picking a rule id, grep the whole tree (`grep -rhoE 'MDL0[0-9]{2}' --include=*.go --include=*.md`) and `git log --all -S'MDLnnn'` — there is no `mxcli help ` and no central list to consult. The guard test now also asserts the old id is never reported", "refs": []} diff --git a/.claude/skills/mendix/generate-domain-model/reference/syntax.md b/.claude/skills/mendix/generate-domain-model/reference/syntax.md index 4aacc5d207..c2bb362726 100644 --- a/.claude/skills/mendix/generate-domain-model/reference/syntax.md +++ b/.claude/skills/mendix/generate-domain-model/reference/syntax.md @@ -519,7 +519,7 @@ type reference; Prefer `if not exists` when the statement is a *delta* rather than the element's complete definition. `or modify` rebuilds the element from the statement, so a partial `create or modify entity` drops every attribute it does not list. Writing -both is refused as **MDL067**. +both is refused as **MDL085**. **Association Types**: - `reference` - One-to-one or many-to-one (foreign key on FROM entity) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e3b5a8f1..d5e4acdd1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`DynamicClasses` and a column's `DynamicCellClass` are written as Mendix expressions** (mendixlabs/mxcli#750) — the expression is written as-is, so the doubled-quote spelling is gone: `dynamicclasses: if $currentObject/Featured then 'is-featured' else ''`, and `dynamicclasses: 'is-featured'` is the string — the class `is-featured`. The same rule as the OData client's credentials. `create page`, `alter page … set` and `describe` all use it, and a describe → exec round trip stores identical values (measured on a Mendix 11.14.0 project). **Migrating a script:** the old spelling, the expression's text in quotes (`'if … then ''a'' else '''''`), still parses but would now store that text as a class name, so `check` and `exec` refuse it as **MDL-WIDGET33** and give the unquoted expression. An expression in any other widget property is an error rather than an empty value; a pluggable property whose schema kind is Expression (a column's `Visible`, for one) keeps the quoted form until a following change. - **An OData client's credentials and header values are written as Mendix expressions** (mendixlabs/mxcli#750) — `HttpUsername`, `HttpPassword`, `ClientCertificate` and every `headers (…)` value hold an expression, and MDL now writes it as-is: `HttpUsername: 'admin'` is the string `'admin'`, `@Module.Const` reads a constant, and `'Bearer ' + @Module.Token` concatenates. Before, a quoted value was the expression's *text*, so `'admin'` stored the identifier `admin` and a string needed `'''admin'''`. `describe` prints the stored expression as-is, so Studio Pro's `'abc'` now reads `HttpUsername: 'abc'`; measured against a Studio Pro-authored client, and a describe → exec round trip stores identical values. **Migrating a script:** `'''admin'''` becomes `'admin'`, and a quoted constant `'@Module.Const'` becomes `@Module.Const` — both old forms still parse but would now store something else, so `check` and `exec` refuse them as **MDL-ODATA07**. A compound expression in any other OData property (`Path: 'a' + 'b'`) is an error rather than an empty value. `ServiceUrl` is a constant reference, not an expression — see the next entry. - **An OData client's `ServiceUrl` names a constant, like `ProxyHost`** (mendixlabs/mxcli#750) — Studio Pro picks the service URL as a constant and stores it as `@Module.Name`. `ServiceUrl: Module.Location` is now accepted alongside `@Module.Location` and `'@Module.Location'` (the bare name used to be refused as "not a constant reference"); all three store the same value, and `describe` prints the bare name, as it does for the proxy references. A literal URL is still refused (CE6825). +- **`create or modify … if not exists` is reported as `MDL085`** — the error had the id `MDL067`, which also names the unrelated bare-commit note (a bare `commit $X;` now runs events). One id for two diagnostics made it useless for looking either one up. The commit note keeps `MDL067`; a CI filter or suppression keyed on `MDL067` for the guard error needs `MDL085`. ### Fixed diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 976b15af25..d769377e30 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -167,7 +167,7 @@ func init() { "-- plain CREATE.\n" + "--\n" + "-- It is not CREATE OR MODIFY, which makes the stored element match the\n" + - "-- statement. Writing both is refused as MDL067. DESCRIBE never emits it.\n" + + "-- statement. Writing both is refused as MDL085. DESCRIBE never emits it.\n" + "--\n" + "-- Every CREATE that names one element accepts it. Not accepted where\n" + "-- there is no one named element to test: ANNOTATION, INDEX (use ALTER\n" + diff --git a/docs-site/src/language/basics.md b/docs-site/src/language/basics.md index f84783ffc1..ca52340712 100644 --- a/docs-site/src/language/basics.md +++ b/docs-site/src/language/basics.md @@ -109,7 +109,7 @@ A plain `create` fails when the element already exists. Two guards make a script | `create or modify page M.P …` | created | rewritten to match the statement (identity kept) | | `create page if not exists M.P …` | created | **left untouched**, reported as skipped | -`if not exists` goes after the kind's keywords and before the name, on every `create` that names one element — `create microflow if not exists M.MF () …`, `create module if not exists M;`, `create user role if not exists Clerk (M.User);`, `create configuration if not exists 'Default' (…);`. It is not accepted on `annotation`, `index` (use `alter entity … add index if not exists`), `validation rule`, `navigation`, `translations` or `external entities`, which have no single named element to test. Writing `create or modify … if not exists` is refused as `MDL067`: the two guards contradict each other. `describe` never emits `if not exists`. +`if not exists` goes after the kind's keywords and before the name, on every `create` that names one element — `create microflow if not exists M.MF () …`, `create module if not exists M;`, `create user role if not exists Clerk (M.User);`, `create configuration if not exists 'Default' (…);`. It is not accepted on `annotation`, `index` (use `alter entity … add index if not exists`), `validation rule`, `navigation`, `translations` or `external entities`, which have no single named element to test. Writing `create or modify … if not exists` is refused as `MDL085`: the two guards contradict each other. `describe` never emits `if not exists`. ## Case Insensitivity diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 8f2c0a7941..2754fabc01 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -141,7 +141,7 @@ Modifies an existing entity without full replacement. > rebuilds the element from the statement and **drops any attribute the statement > omits**, so it is only safe when the statement is the element's complete > definition. Writing both (`create or modify … if not exists`) is refused as -> **MDL067**. +> **MDL085**. > > To re-run a whole script that is partly applied and not guarded, use > `mxcli exec script.mdl --continue-on-error`: every statement is attempted, each diff --git a/mdl/ast/ast_create_guard.go b/mdl/ast/ast_create_guard.go index 9f334f441f..d26241d248 100644 --- a/mdl/ast/ast_create_guard.go +++ b/mdl/ast/ast_create_guard.go @@ -17,7 +17,7 @@ type CreateGuard struct { IfNotExists bool // GuardWithOrModify records that the same statement also said // `create or modify` (or its alias `or replace`). The two guards contradict - // each other, which check reports as MDL067. + // each other, which check reports as MDL085. GuardWithOrModify bool } diff --git a/mdl/executor/cmd_create_guard.go b/mdl/executor/cmd_create_guard.go index f6732b2c75..23c9198ef8 100644 --- a/mdl/executor/cmd_create_guard.go +++ b/mdl/executor/cmd_create_guard.go @@ -289,7 +289,7 @@ func documentListed(ctx *ExecContext, items any, qn ast.QualifiedName) (bool, er } // validateCreateGuardContradiction reports `create or modify … if not exists` -// (MDL067) on every guarded document kind. The two guards contradict each +// (MDL085) on every guarded document kind. The two guards contradict each // other: `or modify` makes the stored element match the statement, `if not // exists` leaves it untouched, and which one wins is not readable from the // statement. Entities and associations report it from their own flags. diff --git a/mdl/executor/cmd_enumerations.go b/mdl/executor/cmd_enumerations.go index 58b68da66a..e2774f21c6 100644 --- a/mdl/executor/cmd_enumerations.go +++ b/mdl/executor/cmd_enumerations.go @@ -731,7 +731,7 @@ func validateEntityAttribute(attr ast.Attribute, kind entityPersistence, entityN return violations } -// validateIdempotencyGuard (MDL067) rejects CREATE OR MODIFY … IF NOT EXISTS. +// validateIdempotencyGuard (MDL085) rejects CREATE OR MODIFY … IF NOT EXISTS. // // Both spellings exist to make a script re-runnable and they mean opposite // things about an existing element: OR MODIFY rebuilds it from the statement, @@ -744,7 +744,7 @@ func validateIdempotencyGuard(createOrModify, ifNotExists bool, kind, name strin return nil } return []linter.Violation{{ - RuleID: "MDL067", + RuleID: "MDL085", Severity: linter.SeverityError, Message: fmt.Sprintf("'create or modify %s ... if not exists' combines two contradictory guards — "+ "'or modify' replaces the stored definition, 'if not exists' leaves it untouched", kind), diff --git a/mdl/executor/validate_idempotency_guard_test.go b/mdl/executor/validate_idempotency_guard_test.go index 247583f38c..a9e7a21699 100644 --- a/mdl/executor/validate_idempotency_guard_test.go +++ b/mdl/executor/validate_idempotency_guard_test.go @@ -10,11 +10,15 @@ import ( "github.com/mendixlabs/mxcli/mdl/visitor" ) -// TestMDL067RejectsContradictoryGuards covers the one combination the grammar +// TestMDL085RejectsContradictoryGuards covers the one combination the grammar // allows and nobody can mean: OR MODIFY rebuilds an existing element from the // statement, IF NOT EXISTS leaves it untouched. Written together one is // silently ignored, and which one is not readable from the statement. -func TestMDL067RejectsContradictoryGuards(t *testing.T) { +// +// The rule was MDL067 until it was found sharing that id with the bare-commit +// note (validate_commit_events.go): one id naming both an error and an +// unrelated info note made the id useless for looking a diagnostic up. +func TestMDL085RejectsContradictoryGuards(t *testing.T) { for _, src := range []string{ `create or modify entity if not exists M."Game" ("Level": string(20));`, `create or modify association if not exists M.Move_Game from M.Move to M.Game;`, @@ -33,12 +37,15 @@ func TestMDL067RejectsContradictoryGuards(t *testing.T) { } var got []string for _, v := range ValidateProgram(prog, "") { - if v.RuleID == "MDL067" { + switch v.RuleID { + case "MDL085": got = append(got, v.Message+" / "+v.Suggestion) + case "MDL067": + t.Errorf("%s\n reported as MDL067, the bare-commit note's id: %s", src, v.Message) } } if len(got) != 1 { - t.Fatalf("%s\n MDL067 fired %d times, want 1", src, len(got)) + t.Fatalf("%s\n MDL085 fired %d times, want 1", src, len(got)) } if !strings.Contains(got[0], "if not exists") || !strings.Contains(got[0], "or modify") { t.Errorf("the message should name both halves, got: %s", got[0]) @@ -46,10 +53,10 @@ func TestMDL067RejectsContradictoryGuards(t *testing.T) { } } -// TestMDL067LeavesEitherGuardAlone is the control. Each spelling on its own is +// TestMDL085LeavesEitherGuardAlone is the control. Each spelling on its own is // the whole point of the feature, so a rule that fires on them would be worse // than no rule. -func TestMDL067LeavesEitherGuardAlone(t *testing.T) { +func TestMDL085LeavesEitherGuardAlone(t *testing.T) { for _, src := range []string{ `create entity if not exists M."Game" ("Level": string(20));`, `create or modify entity M."Game" ("Level": string(20));`, @@ -66,8 +73,8 @@ func TestMDL067LeavesEitherGuardAlone(t *testing.T) { t.Fatalf("%s\n parse errors: %v", src, errs) } for _, v := range ValidateProgram(prog, "") { - if v.RuleID == "MDL067" { - t.Errorf("MDL067 fired on a valid statement:\n %s\n %s", src, v.Message) + if v.RuleID == "MDL085" { + t.Errorf("MDL085 fired on a valid statement:\n %s\n %s", src, v.Message) } } } diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 484912c802..2574fe03c2 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -39,7 +39,7 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { if alterStmt, ok := stmt.(*ast.AlterEntityStmt); ok { violations = append(violations, ValidateAlterEntity(alterStmt)...) } - // An association carries the same pair of contradictory guards (MDL067), + // An association carries the same pair of contradictory guards (MDL085), // and its FROM entity must live in the module it is declared in (MDL070) — // the remote-parent form writes a project that cannot be opened. if assocStmt, ok := stmt.(*ast.CreateAssociationStmt); ok { diff --git a/mdl/visitor/visitor_create_if_not_exists_test.go b/mdl/visitor/visitor_create_if_not_exists_test.go index 12303e3d9f..d66b76c1a1 100644 --- a/mdl/visitor/visitor_create_if_not_exists_test.go +++ b/mdl/visitor/visitor_create_if_not_exists_test.go @@ -108,7 +108,7 @@ func TestCreateIfNotExistsOnEveryDocumentKind(t *testing.T) { } // `create or modify … if not exists` is recorded as such on every kind, so -// check can refuse it (MDL067) — the two guards contradict each other. +// check can refuse it (MDL085) — the two guards contradict each other. func TestCreateOrModifyIfNotExistsIsRecorded(t *testing.T) { for name, body := range createOrReplaceCases { if _, exempt := createIfNotExistsExempt[name]; exempt { From 4a9815907ab938ec1848566b05ca26e4864ea4c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:02:33 +0000 Subject: [PATCH 15/60] fix(exec): drop the MDL067 note for a flow already stored that way The bare-commit note ("a bare `commit $X;` now writes WITH EVENTS; it used to write them off") printed on every `exec` of an idempotent script, even on a re-run reporting the flows unchanged, where the stored commits already have WithEvents=true. It buried the warnings that apply. The note is a script-only check and ValidateProgram has no backend, so the filter runs in execPreflight, which holds the connected executor: DropSettledCommitNotes asks StoredCommitEvents (built for fmt --upgrade -p) for the stored flags and drops the note only when every variable committed bare is stored with the same number of commits with and without events. A flow not stored yet, a plain create, an added commit or a stored commit without events keeps it. `check` is unchanged. Measured on a Verify copy: run 2 of a two-flow script prints no MDL067; after storing one flow `without events`, run 3 notes only that flow. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + cmd/mxcli/exec_preflight.go | 6 + docs-site/src/language/versions.md | 2 +- mdl/executor/validate_commit_events.go | 9 +- .../validate_commit_events_settled.go | 106 +++++++++++++++ .../validate_commit_events_settled_test.go | 124 ++++++++++++++++++ 7 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 mdl/executor/validate_commit_events_settled.go create mode 100644 mdl/executor/validate_commit_events_settled_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 3cc8a08a09..d40792a546 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -801,3 +801,4 @@ {"date": "2026-10-01", "area": "mdl/executor", "symptom": "ako/mxcli#571 / mendixlabs/mxcli#1206: `create published rest service` wrote only the path's {name} placeholders as operation parameters, each a String: every query and body microflow parameter failed mx check with CE0350, an Integer {id} with CE6539. `import mapping` / `export mapping` / `commit` on an operation parsed and were thrown away (CE0350 on the body, CE0354 on an object-returning microflow). Executing describe of a Studio Pro service (TestApp Services.OrdersRestApi) reported 'Modified' and broke a 0-error app with 5 errors: mappings cleared, Integer path params retyped String, body param dropped, Commit No->Yes, Basic+Session authentication turned off", "cause": "publishedRestOperationToGen built parameters from the path alone and wrote ExportMapping/ImportMapping \"\" and Commit \"Yes\" as constants; the reader never read parameters, mappings or commit, so describe could not print them and ALTER (which rewrites every operation) lost them too; the service writer also emitted constants for AuthenticationTypes / AuthenticationMicroflow / CorsConfiguration / Documentation / PublicDocumentation with no carry; Resources and operation Parameters were registered with list marker 2 where Studio Pro writes 3", "file": "mdl/executor/cmd_published_rest.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/export_level_carry.go, mdl/visitor/visitor_rest.go, model/types.go", "fix": "the executor derives operation parameters from the microflow as Studio Pro does (path name -> Path, object/list -> Body, System.HttpRequest/HttpResponse -> none, else Query; the microflow parameter's type), merged over the stored parameters per bound microflow parameter so a header/renamed/described parameter survives; mappings and commit flow AST -> model -> BSON and back, describe prints them (commit when not Yes) and notes parameters MDL cannot state; an unknown commit value is refused at exec and by check (MDL-REST03); create or modify carries summary/documentation/object handling of the restated operation; UpdatePublishedRestService carries the stored service-level keys MDL cannot state (keepStoredTopLevel); list markers measured from TestApp", "test": "mdl/executor/cmd_published_rest_params_test.go; mdl/backend/modelsdk/published_rest_write_test.go TestCreatePublishedRestService_WritesParametersAndBindings, TestWithStoredTopLevel; mdl/roundtrip TestTestAppRoundTrip/published_rest_service_Services.OrdersRestApi (allowlist entry struck); mdl-examples/bug-tests/571-published-rest-parameters-and-mappings.mdl (TestApp copy: old binary 16 mx check errors, fixed 0, describe->exec Unchanged twice)", "insight": "A clause that parses and is then ignored is worse than a parse error: the grammar advertised import/export mapping for months while the writer hard-coded them empty. The fastest witness was the round-trip harness's own allowlist entry for the one Studio Pro published REST service in TestApp: removing it printed the whole loss set (bindings, parameter types, markers, authentication) in one diff. Studio Pro's metamodel (ped_get_schema over the MCP tunnel) gave the enum values and defaults: Commit defaults to No there, while mxcli keeps writing Yes when the clause is absent so existing scripts do not churn, and describe prints commit whenever it is not Yes."} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "Nightly, Mendix 10.24 only: `TestMxCheck_DoctypeScripts/24-workflow-examples.mdl/modelsdk` → `skipped 481 version-gated lines` then `Execution error: entity 'WFTest.OrderContext' not found for parameter 'OrderContext'` — the same failure TestFilterByVersion_FileBaselineSurvivesAny was written for, back again with that test green", "cause": "`filterByVersion` treats a `-- @version:` directive as the file's floor only if no statement precedes it. #901 put `mdl 1;` on line 1 of every doctype script, above 24's `-- @version: 11.0+`; the header counted as a statement, the floor was lost, and PART H's `-- @version: any` re-enabled a section whose fixtures (WFTest.OrderContext, 11.0+) had been skipped", "file": "`mdl/executor/roundtrip_doctype_test.go` (filterByVersion)", "insight": "The language header is not a statement: `langver.IsHeaderLine` excludes it. The earlier regression test used a synthetic script without a header, so a corpus-wide header migration could not trip it — the new guard also runs filterByVersion on the real 24-workflow-examples.mdl. When a script-format migration lands, re-run the line-oriented tooling that reads those scripts (version gating, skip lists) against the real files, not synthetic ones. Repro: `MX_BINARY=~/.mxcli/mxbuild/10.24.24.119349/modeler/mx go test -tags integration -run TestMxCheck_DoctypeScripts/24-workflow ./mdl/executor/` (fails with exactly 481 skipped lines without the fix, 523 with it)", "refs": ["#901"]} {"area": "mdl/executor", "date": "2026-10-02", "symptom": "`MDL067` names two unrelated diagnostics: the ERROR for `create or modify … if not exists` (contradictory guards) and the INFO note that a bare `commit $X;` now runs events. A user filtering or looking up MDL067 cannot tell which one they have", "cause": "Rule ids are string literals at each `addViolation` / `RuleID:` site with no registry, so a later rule picked an id already in use and nothing failed", "file": "`mdl/executor/cmd_enumerations.go` (validateIdempotencyGuard), `mdl/executor/cmd_create_guard.go`, `mdl/executor/validate_commit_events.go`", "insight": "The guard error moved to MDL085 (a gap no branch's history ever used); the commit note kept MDL067 because `fmt --upgrade`, mdl/upgrade and the released CHANGELOG already name it for that note. Before picking a rule id, grep the whole tree (`grep -rhoE 'MDL0[0-9]{2}' --include=*.go --include=*.md`) and `git log --all -S'MDLnnn'` — there is no `mxcli help ` and no central list to consult. The guard test now also asserts the old id is never reported", "refs": []} +{"area": "mdl/executor", "date": "2026-10-02", "symptom": "Re-running an idempotent `mdl 1` script with `exec -p` prints the MDL067 note (\"1 commit activity uses the default, which is now WITH EVENTS…\") for every microflow with a bare `commit $x;`, on every run — including a run that reports \"N documents already in sync (unchanged)\", where the stored flow already has WithEvents=true", "cause": "checkBareCommitEvents is a script-only check (ValidateProgram has a path, not a backend), so it cannot tell a flow whose stored commits would flip from one this script already wrote. execPreflight printed it unfiltered", "file": "`mdl/executor/validate_commit_events_settled.go` (DropSettledCommitNotes), `cmd/mxcli/exec_preflight.go`", "insight": "Filter after validation where a backend exists, rather than threading a backend into ValidateProgram: execPreflight holds the connected executor, and `StoredCommitEvents` (built for fmt --upgrade -p, ako/mxcli#873) already answers the per-variable stored WithEvents flags. Suppress only on exact per-variable multiset equality (bare = with events) — not found, plain create, extra commits or a stored `without` keep the note. `check` is unchanged: it prints violations before it connects. E2E on a Verify copy: run 2 of a two-flow script is silent; after storing one flow `without events`, only that flow is noted", "refs": []} diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e4acdd1f..ccf385f33c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **An OData client's credentials and header values are written as Mendix expressions** (mendixlabs/mxcli#750) — `HttpUsername`, `HttpPassword`, `ClientCertificate` and every `headers (…)` value hold an expression, and MDL now writes it as-is: `HttpUsername: 'admin'` is the string `'admin'`, `@Module.Const` reads a constant, and `'Bearer ' + @Module.Token` concatenates. Before, a quoted value was the expression's *text*, so `'admin'` stored the identifier `admin` and a string needed `'''admin'''`. `describe` prints the stored expression as-is, so Studio Pro's `'abc'` now reads `HttpUsername: 'abc'`; measured against a Studio Pro-authored client, and a describe → exec round trip stores identical values. **Migrating a script:** `'''admin'''` becomes `'admin'`, and a quoted constant `'@Module.Const'` becomes `@Module.Const` — both old forms still parse but would now store something else, so `check` and `exec` refuse them as **MDL-ODATA07**. A compound expression in any other OData property (`Path: 'a' + 'b'`) is an error rather than an empty value. `ServiceUrl` is a constant reference, not an expression — see the next entry. - **An OData client's `ServiceUrl` names a constant, like `ProxyHost`** (mendixlabs/mxcli#750) — Studio Pro picks the service URL as a constant and stores it as `@Module.Name`. `ServiceUrl: Module.Location` is now accepted alongside `@Module.Location` and `'@Module.Location'` (the bare name used to be refused as "not a constant reference"); all three store the same value, and `describe` prints the bare name, as it does for the proxy references. A literal URL is still refused (CE6825). - **`create or modify … if not exists` is reported as `MDL085`** — the error had the id `MDL067`, which also names the unrelated bare-commit note (a bare `commit $X;` now runs events). One id for two diagnostics made it useless for looking either one up. The commit note keeps `MDL067`; a CI filter or suppression keyed on `MDL067` for the guard error needs `MDL085`. +- **`exec -p` no longer repeats the bare-commit note (`MDL067`) on a re-run** — for a `create or modify microflow` whose stored flow already commits each variable the way the script writes it (a bare commit counting as with events), re-running changes nothing about events, so exec drops the note; it printed on every run of an idempotent script, burying the warnings that apply. A flow not stored yet, a plain `create`, or a stored commit without events that the bare one would flip still gets it, and `check` still names every flow with a bare commit. ### Fixed diff --git a/cmd/mxcli/exec_preflight.go b/cmd/mxcli/exec_preflight.go index 61432cdfac..ec84bb58f3 100644 --- a/cmd/mxcli/exec_preflight.go +++ b/cmd/mxcli/exec_preflight.go @@ -26,6 +26,12 @@ func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath strin // model. Warnings are printed and do not stop the run. if !skipCheck { violations := executor.ApplyDeprecationPolicy(executor.ValidateProgram(prog, projectPath), depPolicy) + // The bare-commit note (MDL067) says a re-run flips what is stored; + // for a flow the project already holds that way it does not, and the + // note would repeat on every run of an idempotent script. + if b := exec.Backend(); b != nil { + violations = executor.DropSettledCommitNotes(violations, prog, executor.NewStoredCommitEvents(b)) + } if len(violations) > 0 { formatter := linter.GetFormatter(linter.OutputFormatText, color) formatter.Format(violations, w) diff --git a/docs-site/src/language/versions.md b/docs-site/src/language/versions.md index 8ec67b858a..3ca6646bcf 100644 --- a/docs-site/src/language/versions.md +++ b/docs-site/src/language/versions.md @@ -206,7 +206,7 @@ different now. Where the old output can still be asked for, the entry says how. | Text-template attribute binding | `{1} = $Order.Total` binds the attribute and renders with its formatting, as Studio Pro stores it, instead of `toString($Order/Total)`. Write `{1} = toString($Order/Total)` to keep the old output. Without the header the binding warns (`MDL-V1-TEMPLATEATTR`). | both | [cmd_pages_template_attr.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/executor/cmd_pages_template_attr.go) | | Gallery row click | A row click on a gallery with single-click selection is refused (`MDL-WIDGET36`), with nothing written, because `mx check` fails such a page; without the header it is written and warns (`MDL-V1-GALLERYCLICK`). | `mdl 1` | [validate_widget_language.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/executor/validate_widget_language.go) | | `create constant … private` | Was never stored. Without the header it parses, does nothing and warns (`MDL-DEPR138`); `fmt --upgrade` deletes it; `mdl 1` refuses it. Set a private value with `mxcli constant set`. | both | [visitor_deprecations.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/visitor/visitor_deprecations.go) | -| Commit events | A bare `commit $X;` commits *with* events, Studio Pro's default; older releases stored it *without*. `check` names the affected flows (`MDL067`), and `fmt --upgrade -p` writes `without events` where the stored flow has it. | both | [commit_events.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/upgrade/commit_events.go), [flow_commit_events.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/executor/flow_commit_events.go) | +| Commit events | A bare `commit $X;` commits *with* events, Studio Pro's default; older releases stored it *without*. `check` names the affected flows (`MDL067`; `exec -p` leaves out a flow the project already stores with events), and `fmt --upgrade -p` writes `without events` where the stored flow has it. | both | [commit_events.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/upgrade/commit_events.go), [flow_commit_events.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/executor/flow_commit_events.go) | | `date` as a type | Mendix has no date-only type: `date` is an alias of `DateTime` and builds exactly what `DateTime` builds. Without the header it warns (`MDL-DEPR160`); `fmt --upgrade` writes `DateTime`; `mdl 1` refuses `date`. | both | [deprecation.go](https://github.com/mendixlabs/mxcli/blob/main/mdl/deprecation/deprecation.go) | ## Reference diff --git a/mdl/executor/validate_commit_events.go b/mdl/executor/validate_commit_events.go index 5bd2c740b5..7e8e1ff07a 100644 --- a/mdl/executor/validate_commit_events.go +++ b/mdl/executor/validate_commit_events.go @@ -9,6 +9,9 @@ import ( "github.com/mendixlabs/mxcli/mdl/linter" ) +// bareCommitNoteRule is the id of the note below. +const bareCommitNoteRule = "MDL067" + // MDL067 is a one-release migration note for #895. // // Before the fix, a bare `commit $X;` stored WithEvents=false. Studio Pro's @@ -34,6 +37,10 @@ import ( // // Only the author knows which they meant, so the note does not guess. It should // be dropped once the release that carries the change is old news. +// +// exec drops it for a flow the project already stores the way the script +// writes it (DropSettledCommitNotes): re-running an idempotent script changes +// nothing about events, so the note would only repeat itself on every run. func (v *microflowValidator) checkBareCommitEvents(body []ast.MicroflowStatement) { bare := 0 var walk func([]ast.MicroflowStatement) @@ -78,7 +85,7 @@ func (v *microflowValidator) checkBareCommitEvents(body []ast.MicroflowStatement if bare == 1 { subject = "1 commit activity uses" } - v.addViolation("MDL067", linter.SeverityInfo, + v.addViolation(bareCommitNoteRule, linter.SeverityInfo, fmt.Sprintf("%s the default, which is now WITH EVENTS to match Studio Pro (#895); "+ "before this release a bare `commit $X;` wrote events OFF", subject), "Write `commit $X without events;` for any commit here that must NOT run its event "+ diff --git a/mdl/executor/validate_commit_events_settled.go b/mdl/executor/validate_commit_events_settled.go new file mode 100644 index 0000000000..0e15deb4d9 --- /dev/null +++ b/mdl/executor/validate_commit_events_settled.go @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// commitEventSource is what DropSettledCommitNotes asks the project: +// StoredCommitEvents, or upgrade.StoredCommits by another name. +type commitEventSource interface { + CommitEvents(nanoflow bool, qualifiedName string) (map[string][]bool, bool) +} + +// DropSettledCommitNotes removes the MDL067 note (validate_commit_events.go) +// for each `create or modify microflow` whose stored flow already commits, per +// variable, exactly what the script's commits will write — a bare commit +// counting as WITH events. +// +// The note tells the author that a bare `commit $X;` now writes the opposite +// of what an older mxcli wrote. For a flow the project already holds that way +// — typically because this very script wrote it on an earlier run — that is +// not true of this run: re-running the script changes nothing about events, +// and exec reports the flow unchanged. Printing the note anyway, on every run +// of an idempotent script, buried the warnings that do apply. +// +// The note stays whenever the run may change what is stored, or cannot be +// told not to: no project (stored == nil), a flow the project does not have +// yet, a plain `create` (its commits are not recorded in prog.FlowCommits — +// it is new code), a stored commit without events that a bare one would flip, +// or a different number of commits of the variable. The matching is the +// same per-variable multiset fmt --upgrade -p uses (mdl/upgrade +// pinCommitEvents), held to equality: anything the upgrade would pin or +// report is left noted here. +func DropSettledCommitNotes(violations []linter.Violation, prog *ast.Program, stored commitEventSource) []linter.Violation { + if stored == nil || prog == nil { + return violations + } + settled := map[string]bool{} + isSettled := func(flow string) bool { + if s, done := settled[flow]; done { + return s + } + s := commitsMatchStored(prog.FlowCommits, flow, stored) + settled[flow] = s + return s + } + out := violations[:0:0] + for _, v := range violations { + if v.RuleID == bareCommitNoteRule && v.Location.DocumentType == "microflow" && + isSettled(v.Location.DocumentName) { + continue + } + out = append(out, v) + } + return out +} + +// commitsMatchStored reports whether every variable the script commits bare +// in flow is committed by the stored flow exactly as the script will write +// it: the same number of commits with events and without. +func commitsMatchStored(commits []ast.FlowCommit, flow string, stored commitEventSource) bool { + type tally struct{ with, without int } + script := map[string]*tally{} + bare := map[string]bool{} + for _, c := range commits { + if c.Nanoflow || c.Flow.String() != flow { + continue + } + t := script[c.Variable] + if t == nil { + t = &tally{} + script[c.Variable] = t + } + if c.WithoutEvents { + t.without++ + } else { + t.with++ // bare or `with events`: both write WithEvents=true + } + if c.Bare { + bare[c.Variable] = true + } + } + if len(bare) == 0 { + return false // nothing recorded for this flow: a plain create + } + events, found := stored.CommitEvents(false, flow) + if !found { + return false + } + for v := range bare { + var have tally + for _, e := range events[v] { + if e { + have.with++ + } else { + have.without++ + } + } + if have != *script[v] { + return false + } + } + return true +} diff --git a/mdl/executor/validate_commit_events_settled_test.go b/mdl/executor/validate_commit_events_settled_test.go new file mode 100644 index 0000000000..18a5c2fa32 --- /dev/null +++ b/mdl/executor/validate_commit_events_settled_test.go @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// storedCommitFlows answers CommitEvents from a fixed table: flow name -> +// variable -> the WithEvents flag of each stored Commit activity. +type storedCommitFlows map[string]map[string][]bool + +func (s storedCommitFlows) CommitEvents(_ bool, qn string) (map[string][]bool, bool) { + ev, ok := s[qn] + return ev, ok +} + +// TestDropSettledCommitNotes pins when exec stays quiet about MDL067: only when +// the stored flow already commits every variable the script commits bare +// exactly as the script will write it. A re-run of an idempotent script then +// changes nothing about events, and the note — "this statement now writes the +// opposite of what it used to" — is false for it. Every other case still +// changes, or may change, what is stored, and keeps the note. +func TestDropSettledCommitNotes(t *testing.T) { + const flow = "M.F" + tests := []struct { + name string + src string + stored storedCommitFlows + wantNote bool + }{ + { + name: "stored with events, as the bare commit writes: settled", + src: "create or modify microflow M.F ($X: M.E) begin commit $X; end;", + stored: storedCommitFlows{flow: {"X": {true}}}, + wantNote: false, + }, + { + name: "explicit and bare commits of one variable all match: settled", + src: "create or modify microflow M.F ($X: M.E) begin commit $X; " + + "commit $X without events; commit $X; end;", + stored: storedCommitFlows{flow: {"X": {false, true, true}}}, + wantNote: false, + }, + { + name: "stored without events — an older mxcli wrote it, the re-run flips it", + src: "create or modify microflow M.F ($X: M.E) begin commit $X; end;", + stored: storedCommitFlows{flow: {"X": {false}}}, + wantNote: true, + }, + { + name: "the flow is not stored yet", + src: "create or modify microflow M.F ($X: M.E) begin commit $X; end;", + stored: storedCommitFlows{}, + wantNote: true, + }, + { + name: "the script adds a bare commit the stored flow does not have", + src: "create or modify microflow M.F ($X: M.E) begin commit $X; commit $X; end;", + stored: storedCommitFlows{flow: {"X": {true}}}, + wantNote: true, + }, + { + name: "one variable settled, another not", + src: "create or modify microflow M.F ($X: M.E, $Y: M.E) begin commit $X; " + + "commit $Y; end;", + stored: storedCommitFlows{flow: {"X": {true}, "Y": {false}}}, + wantNote: true, + }, + { + name: "a plain create is new code, whatever is stored", + src: "create microflow M.F ($X: M.E) begin commit $X; end;", + stored: storedCommitFlows{flow: {"X": {true}}}, + wantNote: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prog, errs := visitor.Build(tt.src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + before := ValidateProgram(prog, "") + // The control: without a project the note always fires, so a + // missing note below is the filter's doing, not the validator's. + if countRule(before, "MDL067") != 1 { + t.Fatalf("control: want one MDL067 before filtering, got %d", countRule(before, "MDL067")) + } + after := DropSettledCommitNotes(before, prog, tt.stored) + if got := countRule(after, "MDL067") == 1; got != tt.wantNote { + t.Errorf("MDL067 present = %v, want %v", got, tt.wantNote) + } + if len(before)-len(after) > 1 { + t.Errorf("dropped %d violations; only the one MDL067 note may go", len(before)-len(after)) + } + }) + } +} + +// TestDropSettledCommitNotesWithoutProject: no stored flows to ask, nothing +// to drop. +func TestDropSettledCommitNotesWithoutProject(t *testing.T) { + prog, errs := visitor.Build("create or modify microflow M.F ($X: M.E) begin commit $X; end;") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + vs := ValidateProgram(prog, "") + if got := DropSettledCommitNotes(vs, prog, nil); countRule(got, "MDL067") != 1 { + t.Errorf("with no stored flows the note must stay") + } +} + +func countRule(vs []linter.Violation, id string) int { + n := 0 + for _, v := range vs { + if v.RuleID == id { + n++ + } + } + return n +} From 67f31b89d1d73f9f110a4719586b231907426677 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:11:58 +0000 Subject: [PATCH 16/60] feat(exec): count the pre-flight's info notes instead of printing each MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec's pre-flight printed every violation of the semantic pass in full, with no severity filter. On report pages MDL-WIDGET15 alone was ~35 info notes a run, burying the warnings and errors that matter on a re-run. exec (and diff, which shares the pre-flight) now prints errors and warnings in full and the info notes as one line: "N info notes not shown — run `mxcli check ` to see them (or pass --verbose)". The text summary counts them, marked "(not shown)", via a new TextFormatter.OmittedInfos, so it no longer reads "0 info" above that line. --verbose on exec and diff prints them in full. `check` is unchanged and still prints every note; exec has no structured diagnostics output, so check --format json|sarif are unaffected. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FgupMwfjsUoszFq2kSn2p2 --- .../skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + CHANGELOG.md | 1 + cmd/mxcli/cmd_diff.go | 3 +- cmd/mxcli/cmd_exec.go | 8 +- cmd/mxcli/exec_preflight.go | 57 ++++++++++-- cmd/mxcli/exec_preflight_test.go | 86 +++++++++++++++++++ cmd/mxcli/main.go | 1 + docs-site/src/tutorial/validation.md | 2 + mdl/linter/output.go | 10 +++ 9 files changed, 158 insertions(+), 11 deletions(-) create mode 100644 cmd/mxcli/exec_preflight_test.go diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 5033f17a65..1b13dfd258 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -139,3 +139,4 @@ {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli check`/`exec`/`fmt`/`diff` fail on a script saved by Windows PowerShell 5.1: a UTF-8 BOM gives `line 1:0 token recognition error at: '\\ufeff'` (an invisible character), UTF-16LE gives a token error on almost every character; the same through stdin and in .test.mdl files", "cause": "Every script reader passed the raw file bytes to the lexer, which reads UTF-8 without a BOM; there was no shared reader (fmt, diff, the multi-file check pass, the test runner and EXECUTE SCRIPT each called os.ReadFile on their own)", "fix": "New mdl/srctext.Decode (strip a leading UTF-8 BOM, decode UTF-16LE/BE by BOM); readMDLSource calls it and fmt, diff and parseScriptSet now read through readMDLSource; testrunner.ParseTestFile and EXECUTE SCRIPT call it directly", "insight": "A BOM also hides a `mdl 1;` header from langver.ScanWrittenHeader, so stripping it in the parser alone would have left the language version wrong: decode where the bytes are read, before anything inspects the text. Enumerate the readers (grep os.ReadFile / io.ReadAll(os.Stdin)), not just the one the report names", "issue": "mendixlabs/mxcli#1253", "file": "mdl/srctext/srctext.go; cmd/mxcli/mdlsource.go", "test": "mdl/srctext/srctext_test.go; cmd/mxcli/mdlsource_encoding_test.go"} {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli -p App.mpr -c \"\"` opens the interactive REPL (a generator spawning mxcli with an open stdin hangs at `mdl>`); `-c \"describe entity System.User; describe entity String; describe entity System.FileDocument\"` stops at statement 2 with `module name is required: objects must be created within a module` and the third statement is silently never run", "cause": "Root Run tested `commands != \"\"` to choose -c over the REPL, so an empty flag value was indistinguishable from no flag; the -c path used ExecuteProgram, which returns the first error without its position, and describe entity/association reached findModule(\"\"), whose message is written for the create path", "fix": "`cmd.Flags().Changed(\"command\")` selects the one-liner path; runCommandLine (cmd/mxcli/oneliner.go) refuses empty input, reports `statement N of M` and how many later statements were not run (via new Executor.ExecuteProgramReportingStop), and takes --continue-on-error like exec; execDescribe names an unqualified entity/association name", "insight": "A flag's zero value is not its absence: use Changed() whenever an empty value must mean something other than not given. Decided semantics: -c is fail-fast like exec (a later statement may depend on an earlier one), but a stop is never silent", "issue": "mendixlabs/mxcli#1218", "file": "cmd/mxcli/oneliner.go; cmd/mxcli/main.go; mdl/executor/executor.go; mdl/executor/executor_query.go", "test": "cmd/mxcli/oneliner_test.go; mdl/executor/describe_unqualified_name_test.go"} {"date": "2026-10-01", "area": "cmd/mxcli", "symptom": "`mxcli lsp --stdio` exits rc=2 with `panic: only file URIs are supported, got mendix-mdl` on textDocument/didOpen of a `mendix-mdl:` virtual document (the VS Code extension's describe previews); VS Code restarts it, it crashes again, and after 5 crashes it stops restarting the server", "cause": "checkableDocument (diagnostics) and CodeAction called go.lsp.dev/uri URI.Filename(), which panics on any scheme but file, to decide whether the document is a .test.mdl", "fix": "documentPath(uri) returns Filename() only for file: URIs and the URI's path component otherwise; runSemanticCheck (which shells out `mxcli check `) skips non-file documents; virtual documents are still diagnosed in memory", "insight": "Third-party helpers that panic on unexpected input are a crash path in a long-running server; every URI an LSP client sends is untrusted shape. A test with a non-file URI plus a file-URI control (same diagnostics) proves the virtual case is handled, not skipped", "issue": "mendixlabs/mxcli#1245", "file": "cmd/mxcli/lsp_helpers.go (documentPath, isFileURI); cmd/mxcli/lsp_diagnostics.go; cmd/mxcli/lsp_language.go", "test": "cmd/mxcli/lsp_virtual_uri_test.go"} +{"area": "cmd/mxcli", "date": "2026-10-02", "symptom": "`mxcli exec` on a script with report pages prints ~35 MDL-WIDGET15 info notes (one per container with adjacent inline dynamictexts) on every run, burying the warnings and errors that matter", "cause": "execPreflight formatted every ValidateProgram violation with linter.TextFormatter, no severity filter — the same report `check` prints, on a command that is re-run", "file": "`cmd/mxcli/exec_preflight.go` (printPreflightViolations), `mdl/linter/output.go` (TextFormatter.OmittedInfos)", "insight": "Filter at the exec printer, not at the rule: `check` is where a script is reviewed and keeps every note, and lowering a rule's frequency would hide it there too. The formatter's summary line had to learn about the omitted notes, or it read `0 info` above the line saying N were hidden. exec has no structured diagnostics output, so JSON/SARIF (check --format) are untouched. `--verbose` on exec and diff restores the full report", "refs": []} diff --git a/CHANGELOG.md b/CHANGELOG.md index ccf385f33c..5998f11eb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **An OData client's `ServiceUrl` names a constant, like `ProxyHost`** (mendixlabs/mxcli#750) — Studio Pro picks the service URL as a constant and stores it as `@Module.Name`. `ServiceUrl: Module.Location` is now accepted alongside `@Module.Location` and `'@Module.Location'` (the bare name used to be refused as "not a constant reference"); all three store the same value, and `describe` prints the bare name, as it does for the proxy references. A literal URL is still refused (CE6825). - **`create or modify … if not exists` is reported as `MDL085`** — the error had the id `MDL067`, which also names the unrelated bare-commit note (a bare `commit $X;` now runs events). One id for two diagnostics made it useless for looking either one up. The commit note keeps `MDL067`; a CI filter or suppression keyed on `MDL067` for the guard error needs `MDL085`. - **`exec -p` no longer repeats the bare-commit note (`MDL067`) on a re-run** — for a `create or modify microflow` whose stored flow already commits each variable the way the script writes it (a bare commit counting as with events), re-running changes nothing about events, so exec drops the note; it printed on every run of an idempotent script, burying the warnings that apply. A flow not stored yet, a plain `create`, or a stored commit without events that the bare one would flip still gets it, and `check` still names every flow with a bare commit. +- **`exec` and `diff` count the pre-flight's info notes instead of printing each one** — errors and warnings still print in full; info notes (`MDL-WIDGET15` alone was ~35 a run on report pages) become one line, `N info notes not shown — run mxcli check to see them`, and the summary reads `… N info (not shown)`. `--verbose` prints them in full. `check` is unchanged, as are `check --format json|sarif`. ### Fixed diff --git a/cmd/mxcli/cmd_diff.go b/cmd/mxcli/cmd_diff.go index 7cced952ab..7cd4d549b1 100644 --- a/cmd/mxcli/cmd_diff.go +++ b/cmd/mxcli/cmd_diff.go @@ -63,6 +63,7 @@ Examples: useColor, _ := cmd.Flags().GetBool("color") width, _ := cmd.Flags().GetInt("width") skipCheck, _ := cmd.Flags().GetBool("no-check") + verbose, _ := cmd.Flags().GetBool("verbose") continueOnError, _ := cmd.Flags().GetBool("continue-on-error") showExecOutput, _ := cmd.Flags().GetBool("exec-output") depPolicy := deprecationPolicy(cmd) @@ -98,7 +99,7 @@ Examples: NewBackend: func() backend.FullBackend { return modelsdkbackend.New() }, ContinueOnError: continueOnError, Preflight: func(scratch *executor.Executor, w io.Writer) string { - return execPreflight(scratch, prog, projectPath, skipCheck, depPolicy, w, useColor) + return execPreflight(scratch, prog, projectPath, filePath, skipCheck, verbose, depPolicy, w, useColor) }, } if filePath != "-" { diff --git a/cmd/mxcli/cmd_exec.go b/cmd/mxcli/cmd_exec.go index 035e37d80c..9cca60e897 100644 --- a/cmd/mxcli/cmd_exec.go +++ b/cmd/mxcli/cmd_exec.go @@ -23,7 +23,8 @@ Before anything is written, the script is put through the same semantic checks as "mxcli check". If any of them reports an error, nothing is executed: exec applies statements one at a time and cannot roll back, so running a script with a known error leaves the model partly updated. Warnings are printed and do not -stop the run. Use --no-check to apply a script anyway. +stop the run; info notes are counted on one line (--verbose prints them, as +"mxcli check" does). Use --no-check to apply a script anyway. A deprecated MDL spelling (MDL-DEPRnnn, e.g. "create or replace" for "create or modify") is a warning; --deprecations=error makes it an error. @@ -60,6 +61,7 @@ Example: projectPath, _ := cmd.Flags().GetString("project") continueOnError, _ := cmd.Flags().GetBool("continue-on-error") skipCheck, _ := cmd.Flags().GetBool("no-check") + verbose, _ := cmd.Flags().GetBool("verbose") if force, _ := cmd.Flags().GetBool("force"); force { mmpr.AllowWritesWhileStudioProOpen = true if lock, _ := mmpr.StudioProLockFile(projectPath); lock != "" { @@ -116,7 +118,7 @@ Example: os.Exit(1) } - if refusal := execPreflight(exec, prog, projectPath, skipCheck, depPolicy, os.Stderr, true); refusal != "" { + if refusal := execPreflight(exec, prog, projectPath, filePath, skipCheck, verbose, depPolicy, os.Stderr, true); refusal != "" { fmt.Fprint(os.Stderr, refusal) os.Exit(1) } @@ -147,6 +149,8 @@ Example: func init() { execCmd.Flags().Bool("no-check", false, "Skip the pre-flight semantic checks and apply the script even if mxcli check would report errors") + execCmd.Flags().Bool("verbose", false, + "Print the pre-flight checks' info notes in full instead of counting them (mxcli check always prints them)") execCmd.Flags().Bool("force", false, "Write even though Studio Pro appears to have the project open (its .mpr.lock is present) — e.g. a lock left behind by a crash") execCmd.Flags().Bool("continue-on-error", false, diff --git a/cmd/mxcli/exec_preflight.go b/cmd/mxcli/exec_preflight.go index ec84bb58f3..b05cc8f1d9 100644 --- a/cmd/mxcli/exec_preflight.go +++ b/cmd/mxcli/exec_preflight.go @@ -18,8 +18,10 @@ import ( // checks, so it refuses exactly the scripts exec refuses (ako/mxcli#807). // // exec is the executor the script would run on, connected to projectPath ("" when -// the script connects itself). -func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath string, skipCheck bool, depPolicy deprecation.Policy, w io.Writer, color bool) string { +// the script connects itself). script names the script for the hint that +// points at `mxcli check` ("-" for stdin). showInfo prints info-level notes in +// full; otherwise they are counted on one line (see printPreflightViolations). +func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath, script string, skipCheck, showInfo bool, depPolicy deprecation.Policy, w io.Writer, color bool) string { // Pre-flight: refuse a script whose semantic checks report an error, // rather than writing part of it and leaving the model to mxbuild. // exec is not transactional, so "run it and see" means a half-applied @@ -29,13 +31,12 @@ func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath strin // The bare-commit note (MDL067) says a re-run flips what is stored; // for a flow the project already holds that way it does not, and the // note would repeat on every run of an idempotent script. - if b := exec.Backend(); b != nil { - violations = executor.DropSettledCommitNotes(violations, prog, executor.NewStoredCommitEvents(b)) - } - if len(violations) > 0 { - formatter := linter.GetFormatter(linter.OutputFormatText, color) - formatter.Format(violations, w) + if exec != nil { + if b := exec.Backend(); b != nil { + violations = executor.DropSettledCommitNotes(violations, prog, executor.NewStoredCommitEvents(b)) + } } + printPreflightViolations(violations, script, showInfo, w, color) if summary := linter.Summarize(violations); summary.Errors > 0 { return fmt.Sprintf( "\nRefusing to execute: %d error(s) above. Nothing was written.\n"+ @@ -118,3 +119,43 @@ func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath strin } return "" } + +// printPreflightViolations prints what exec's semantic pass found: errors and +// warnings in full, info notes as one count line unless showInfo is set. +// +// An info note never stops exec and asks nothing of a script that is being +// re-run; printed in full on every run they buried the warnings that do (on +// report pages MDL-WIDGET15 alone was ~35 notes a run). `check` is where a +// script is reviewed, and it still prints every note, so the count line +// points there. Only this text report changes: exec has no structured +// diagnostics output, and `check --format json|sarif` is untouched. +func printPreflightViolations(violations []linter.Violation, script string, showInfo bool, w io.Writer, color bool) { + shown := violations + infos := 0 + if !showInfo { + shown = nil + for _, v := range violations { + if v.Severity == linter.SeverityInfo { + infos++ + continue + } + shown = append(shown, v) + } + } + if len(shown) > 0 { + // The summary line counts the omitted notes too, marked not shown. + f := &linter.TextFormatter{UseColor: color, OmittedInfos: infos} + f.Format(shown, w) + } + if infos > 0 { + noun := "info notes" + if infos == 1 { + noun = "info note" + } + if script == "" { + script = "