From d9cadfa0af3f0d0592e8cd0257aa2a155e340e61 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:44:49 +0000 Subject: [PATCH 01/10] fix(odata): set the two defaults a published service needs to build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A published OData service created purely from MDL passed `mxcli check` and then failed the build on two counts, neither of which the author could see coming. CE0729 "The service name should not be empty": `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties, and CREATE set only the first. The consumed path has defaulted the same field to the document name for CE0339 all along. CE7375 "Attribute ID ... must be published and be the key when associations are exposed as an associated object id", firing even with no associations exposed: PublishAssociations defaults to false, which is object-id mode, and Mendix only allows that when the system ID is published as the key — while MDL's `expose (Attr (KEY))` publishes an ordinary attribute. The finding framed the second as a non-persistable-entity problem. It is not: measured on 11.12.1, the identical service over a PERSISTENT entity with a unique key builds 0 errors with true and CE7375 with false. The default broke every published service; non-persistable is only where no workaround exists, because publishing the ID of a non-persistable entity is forbidden. So default an unspecified PublishAssociations to true. That is not a preference, it is the only value that can build from the MDL people write. An explicit false is still honoured (tracked separately from "absent" so it survives), warned about when a published entity is non-persistable, and `create or modify` no longer flips a stored value the script never mentioned. Nothing in-repo caught either default: doctype-tests/10-odata-examples.mdl sets both properties explicitly. Both bug-test scripts now build 0 errors on 11.12.1 with no workarounds — no ServiceName, no follow-up ALTER. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .../bug-tests/f1-10.1-odata-service-name.mdl | 55 ++++++++++ .../f1-10.4-odata-publish-associations.mdl | 74 +++++++++++++ mdl/ast/ast_odata.go | 34 +++--- mdl/executor/cmd_odata.go | 96 ++++++++++++++++- .../cmd_odata_publish_associations_test.go | 78 ++++++++++++++ mdl/executor/cmd_odata_service_name_test.go | 101 ++++++++++++++++++ mdl/visitor/visitor_odata.go | 1 + 8 files changed, 423 insertions(+), 17 deletions(-) create mode 100644 mdl-examples/bug-tests/f1-10.1-odata-service-name.mdl create mode 100644 mdl-examples/bug-tests/f1-10.4-odata-publish-associations.mdl create mode 100644 mdl/executor/cmd_odata_publish_associations_test.go create mode 100644 mdl/executor/cmd_odata_service_name_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index abcf12831..25eff01f9 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -391,3 +391,4 @@ extracting `OffsetExpression`/`LimitExpression`. | A fresh clone of a project created by `mxcli new` goes dirty the first time anyone builds it: ~50 **tracked** files modified that nobody edited — every `javascriptsource/*/actions/*.js` gains a banner, `import { Big } from "big.js"` and `export async function`, plus the matching `javasource` stubs. In a cloud session with a stop-hook git check it reads as "uncommitted changes" at the end of clean work | The template ships the generated action stubs in a slightly older shape and MxBuild rewrites them all on the first build. `mx check` does **not** — only a build does — so nothing before the first `run --local` could reveal it, which is after the user has already committed | `cmd/mxcli/docker/settle.go` (`SettleGeneratedSources`), `cmd/mxcli/cmd_new.go` (step 5/6, `--skip-build`), `cmd/mxcli/init.go` (`/theme-cache/` in the generated ignore list) | Fix the *timing*, not the content: run the build while the project is still being created, so the settled form lands in the first commit. Do **not** reimplement the rewrite — it is mxbuild's generator and version-specific; run the real thing. Best-effort by contract (no JDK, no mxbuild, failed build → warning, never a failed creation), because a settled tree is a nicety and a usable project is the deliverable. The other half is gitignore: `theme-cache/` is a cache and says so. A/B on 11.12.1, both git-init'd then built: `--skip-build` → 50 dirty files, default → **0**. Tests `cmd/mxcli/docker/settle_test.go`. mxcli-todo #7 | | `mxcli check` reports `✓ Syntax OK` / `Check passed!`, then `mxcli exec` on the same script fails partway through with `failed to resolve page: page not found: Module.Page` — a button targeting a page the script creates further down. `exec` is not transactional, so the statements before the failure are already written to the .mpr | Page references are resolved in statement order at exec time. `check --references` already had an ordered pass (`validateForwardPageRefs`), but it needs `-p`; plain `check` had nothing, and plain `check` is what gets run | `mdl/executor/validate_page_order.go` (`ValidateScriptPageOrder`, MDL-PAGE01), wired in `cmd/mxcli/cmd_check.go` beside the other project-free validators | The soundness argument is what makes this work without a project: a **plain** CREATE later in the script would fail if the page already existed, so the script itself asserts the page does not exist yet and the earlier reference cannot resolve against the project either. `CREATE OR MODIFY`/`OR REPLACE` assert nothing, so they stay with `--references`, which can look. **Generalisable**: an ordering rule that seems to need project state often does not, once you find the statement that already asserts what you were going to look up. Two things the diagnostic must say and does: a cycle cannot be fixed by ordering (create one page without the linking widget, add it with `ALTER PAGE … INSERT`), and commit before executing a large script. Verified against all `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_page_order_test.go`, example `mdl-examples/bug-tests/todo-9-forward-page-reference.fail.mdl`. mxcli-todo #9 | | A sidebar navigation label renders truncated — "All tasks" shows as "All task" — under every theme (`signal`, `ledger`, `console`) and both variants, so switching theme does not help. Measured on the live DOM as `scrollWidth=56` inside `clientWidth=48` | Atlas's **closed** sidebar is an icon rail: `--navsidebar-width-closed: 48px` in Atlas's own `themesource/atlas_core/web/themes/_theme-default.scss`. The label is wider than the rail, and the closed rail expects an *icon*, not text. No mxcli theme sets any navigation width — the themes map colours — which is exactly why every theme reproduces it | `.claude/skills/mendix/theme-styling.md` (documented; no code change) | **A fix was written, verified, and reverted** — record the reason: adding `text-overflow: ellipsis` to the nav item helps only where Atlas also sets `white-space: nowrap`; elsewhere the label wraps to two readable lines, and the rule turns `All / tasks` into `All / t…`. Screenshotted both ways against the real compiled CSS. The answer belongs to the app (give nav items icons — what the rail is for — or keep the sidebar open), not to a theme that would impose it on every app. **Generalisable**: when a reported symptom traces to an upstream layout constant, reproduce the geometry against the real compiled CSS (48px container, real class names, Playwright measurement) — it takes minutes, tells you whose constant it is, and shows when the "obvious" CSS fix is a regression. mxcli-todo #19d | +| A published OData service created purely from MDL passes `mxcli check` and then fails the build — `[CE0729] "The service name should not be empty."` and `[CE7375] "Attribute ID for entity 'X' must be published and be the key when associations are exposed as an associated object id."` — the second firing even with no associations exposed at all | Two defaults `CREATE ODATA SERVICE` never set. (1) `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties and only the first was set; the CONSUMED path had defaulted this for CE0339 all along. (2) `PublishAssociations` defaults to false = "associations as an associated object id", which Mendix only allows when the system `ID` is published as the key — but MDL's `expose (Attr (KEY))` publishes an ordinary attribute | `mdl/executor/cmd_odata.go` (`serviceName` fallback + heal on create-or-modify; `publishAssociationsFor`; `nonPersistablePublishedEntities` warning), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`PublishAssociationsSet`) | **Wider than reported**: the finding framed CE7375 as a non-persistable-entity problem. Measured on 11.12.1, the identical service with a PERSISTENT entity and a unique key builds 0 errors with `true` and CE7375 with `false` — so the default broke *every* published service, and non-persistable was just where it could not be worked around. Defaulting to true does not pick a preference; it picks the only value that can build from the MDL people write. Needs tri-state (`PublishAssociationsSet`) so an explicit `false` is still honoured, and `create or modify` no longer flips a stored value the author did not mention. **Why nothing caught it**: `mdl-examples/doctype-tests/10-odata-examples.mdl` sets both properties explicitly, so the repo's own example worked around both defaults. Tests `cmd_odata_service_name_test.go`, `cmd_odata_publish_associations_test.go`; examples `f1-10.1-…`, `f1-10.4-…`. mxcli-formula1 #10.1/#10.4 | diff --git a/mdl-examples/bug-tests/f1-10.1-odata-service-name.mdl b/mdl-examples/bug-tests/f1-10.1-odata-service-name.mdl new file mode 100644 index 000000000..c52af4ec1 --- /dev/null +++ b/mdl-examples/bug-tests/f1-10.1-odata-service-name.mdl @@ -0,0 +1,55 @@ +-- ============================================================================ +-- mxcli-formula1 finding #10.1: every published OData service failed to build +-- ============================================================================ +-- +-- Symptom (before fix): a service created purely from MDL passed `mxcli check` +-- and then failed the build: +-- +-- [error] [CE0729] "The service name should not be empty." +-- at Published OData service 'ProbeOData.ProbeApi' +-- +-- `Name` (the document name) and `ServiceName` (the name in the OData metadata +-- document) are different properties, and CREATE ODATA SERVICE only set the +-- first. So the failure hit EVERY published service, not just the exotic ones +-- — and it was invisible until a build ran, because check does not resolve it. +-- +-- The consumed path had defaulted the same field to the document name for +-- CE0339 all along; the published path just never got the same line. +-- +-- After fix: ServiceName falls back to the document name. An explicit +-- `ServiceName: 'X'` still wins. `create or modify` on a service written before +-- the fix heals an empty one, so re-running a script repairs the model. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/f1-10.1-odata-service-name.mdl -p app.mpr +-- then a real build must report 0 errors: +-- scripts/mx-check.sh -p app.mpr +-- ============================================================================ + +create module F1SVCNAME; + +create persistent entity F1SVCNAME.Driver ( + -- The published key needs a unique validation rule, or Mendix answers + -- CE6624 "Add a unique validation rule to attribute 'Code' … to be able to + -- use it as the key". + Code: string(10) unique error 'Code must be unique', + Name: string(120) +); + +-- No ServiceName: — the shape that used to fail CE0729. +create odata service F1SVCNAME.DriverApi ( + path: 'odata/f1svcname/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'F1SVCNAME.Drivers' +) +authentication basic +{ + publish entity F1SVCNAME.Driver as 'Drivers' ( + ReadMode: source + ) + expose ( + Code as 'code' (KEY, Filterable, Sortable), + Name (Filterable, Sortable) + ); +}; diff --git a/mdl-examples/bug-tests/f1-10.4-odata-publish-associations.mdl b/mdl-examples/bug-tests/f1-10.4-odata-publish-associations.mdl new file mode 100644 index 000000000..24d86d6e1 --- /dev/null +++ b/mdl-examples/bug-tests/f1-10.4-odata-publish-associations.mdl @@ -0,0 +1,74 @@ +-- ============================================================================ +-- mxcli-formula1 finding #10.4: PublishAssociations defaulted to unbuildable +-- ============================================================================ +-- +-- Symptom (before fix): a published service failed the build with +-- +-- [error] [CE7375] "Attribute ID for entity 'X' must be published and be the +-- key when associations are exposed as an associated object id." +-- +-- even with NO associations exposed at all. PublishAssociations=false means +-- "expose associations as an associated object id", and Mendix then requires +-- the system ID attribute to be published as the entity key. MDL's +-- `expose (Attr (KEY))` publishes an ordinary attribute, so the old default of +-- false could not build — and for a non-persistable entity nothing could fix +-- it, because publishing the ID of a non-persistable entity is forbidden. +-- +-- Wider than first reported: this was never specific to non-persistable +-- entities. Measured on 11.12.1 with a PERSISTENT entity and a unique key, the +-- identical service builds 0 errors with true and CE7375 with false. +-- +-- After fix: an unspecified PublishAssociations defaults to true (links). An +-- explicit `PublishAssociations: false` is still honoured — and warned about +-- when a published entity is non-persistable, where it can never build. +-- +-- This script is also the reporter's architecture in miniature: a +-- non-persistable entity fed by a read microflow, published over OData v4, with +-- no copy of the data into the database. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/f1-10.4-odata-publish-associations.mdl -p app.mpr +-- then a real build must report 0 errors: +-- scripts/mx-check.sh -p app.mpr +-- ============================================================================ + +create module F1PUBASSOC; + +-- Non-persistable: the rows are produced per request, never stored. +create non-persistent entity F1PUBASSOC.Lap ( + LapKey: string(60), + Driver: string(120), + LapTime: decimal +); + +-- A read microflow backs the published entity set. The $Response parameter is +-- required because the published resource is Countable — Mendix asks the +-- microflow for the count. +CREATE MICROFLOW F1PUBASSOC.Read_Laps ($Response: System.ODataResponse) + RETURNS List of F1PUBASSOC.Lap AS $Laps +BEGIN + $Laps = CREATE LIST OF F1PUBASSOC.Lap; + RETURN $Laps; +END; + +-- No ServiceName:, no PublishAssociations:, no follow-up ALTER — one statement. +create odata service F1PUBASSOC.LapApi ( + path: 'odata/f1laps/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'F1PUBASSOC.Laps' +) +authentication basic +{ + publish entity F1PUBASSOC.Lap as 'Laps' ( + ReadMode: microflow F1PUBASSOC.Read_Laps, + InsertMode: not_supported, + UpdateMode: not_supported, + DeleteMode: not_supported + ) + expose ( + LapKey as 'lapKey' (KEY, Filterable, Sortable), + Driver (Filterable, Sortable), + LapTime (Sortable) + ); +}; diff --git a/mdl/ast/ast_odata.go b/mdl/ast/ast_odata.go index 36bde4eae..3eda46cc5 100644 --- a/mdl/ast/ast_odata.go +++ b/mdl/ast/ast_odata.go @@ -70,20 +70,26 @@ func (s *DropODataClientStmt) isStatement() {} // CreateODataServiceStmt represents: CREATE ODATA SERVICE Module.Name (...) AUTHENTICATION ... { ... } type CreateODataServiceStmt struct { - Name QualifiedName - Path string - Version string - ODataVersion string - Namespace string - ServiceName string - Summary string - Description string - Documentation string - Folder string // Folder path within module (e.g., "Integration/APIs") - PublishAssociations bool - AuthenticationTypes []string - Entities []*PublishedEntityDef - CreateOrModify bool // True if CREATE OR MODIFY was used + Name QualifiedName + Path string + Version string + ODataVersion string + Namespace string + ServiceName string + Summary string + Description string + Documentation string + Folder string // Folder path within module (e.g., "Integration/APIs") + // PublishAssociations selects how associations appear in the metadata: + // true = as links, false = as an associated object id. The executor + // defaults an unspecified value to true, so PublishAssociationsSet records + // whether the author said anything at all — an explicit false is the + // author's choice and is written as given. + PublishAssociations bool + PublishAssociationsSet bool + AuthenticationTypes []string + Entities []*PublishedEntityDef + CreateOrModify bool // True if CREATE OR MODIFY was used } func (s *CreateODataServiceStmt) isStatement() {} diff --git a/mdl/executor/cmd_odata.go b/mdl/executor/cmd_odata.go index 3dbfd1934..ee5f241e2 100644 --- a/mdl/executor/cmd_odata.go +++ b/mdl/executor/cmd_odata.go @@ -1320,6 +1320,10 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro } if stmt.ServiceName != "" { svc.ServiceName = stmt.ServiceName + } else if svc.ServiceName == "" { + // Heal a service written before ServiceName was defaulted: + // re-running the script repairs a model that cannot build. + svc.ServiceName = svc.Name } if stmt.Summary != "" { svc.Summary = stmt.Summary @@ -1327,7 +1331,9 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro if stmt.Description != "" { svc.Description = stmt.Description } - svc.PublishAssociations = stmt.PublishAssociations + if stmt.PublishAssociationsSet { + svc.PublishAssociations = stmt.PublishAssociations + } if len(stmt.AuthenticationTypes) > 0 { svc.AuthenticationTypes = stmt.AuthenticationTypes } @@ -1353,6 +1359,17 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro containerID = folderID } + // Name (the document) and ServiceName (the name in the OData metadata + // document) are different properties, and Mendix requires the second to be + // non-empty — an empty one fails the build with CE0729 "The service name + // should not be empty", which `mxcli check` cannot see. Default it to the + // document name, exactly as the CONSUMED path does for CE0339 above. + // (mxcli-formula1 findings #10.1.) + serviceName := stmt.ServiceName + if serviceName == "" { + serviceName = stmt.Name.Name + } + newSvc := &model.PublishedODataService{ ContainerID: containerID, Name: stmt.Name.Name, @@ -1361,10 +1378,10 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro Version: stmt.Version, ODataVersion: stmt.ODataVersion, Namespace: stmt.Namespace, - ServiceName: stmt.ServiceName, + ServiceName: serviceName, Summary: stmt.Summary, Description: stmt.Description, - PublishAssociations: stmt.PublishAssociations, + PublishAssociations: publishAssociationsFor(stmt), AuthenticationTypes: stmt.AuthenticationTypes, } @@ -1378,6 +1395,18 @@ func createODataService(ctx *ExecContext, stmt *ast.CreateODataServiceStmt) erro newSvc.EntitySets = append(newSvc.EntitySets, entitySet) } + // An explicit false on a non-persistable entity is unbuildable whatever the + // key is: object-id mode needs a published ID, and Mendix forbids publishing + // the ID of a non-persistable entity. Say so rather than let CE7375 be the + // first anyone hears of it. + if !newSvc.PublishAssociations { + if nonPersistable := nonPersistablePublishedEntities(ctx, stmt.Entities); len(nonPersistable) > 0 { + fmt.Fprintf(ctx.Output, + " Warning: PublishAssociations is false, but %s %s non-persistable — associations-as-object-id requires a published ID, which Mendix forbids there. The build will fail with CE7375; remove the property to get the default (true).\n", + strings.Join(nonPersistable, ", "), pluralIsAre(len(nonPersistable))) + } + } + if err := ctx.Backend.CreatePublishedODataService(newSvc); err != nil { return mdlerrors.NewBackend("create OData service", err) } @@ -1782,3 +1811,64 @@ func fetchODataMetadata(metadataUrl string) (metadata string, hash string, err e } // Executor wrappers for unmigrated callers. +// nonPersistablePublishedEntities returns the qualified names of the published +// entities that are non-persistable, in statement order. Entities it cannot +// resolve are treated as persistable: this drives a silent default correction, +// so an unreadable entity must not change what gets written. +func nonPersistablePublishedEntities(ctx *ExecContext, defs []*ast.PublishedEntityDef) []string { + if ctx == nil || ctx.Backend == nil { + return nil + } + var out []string + seen := make(map[string]bool) + for _, def := range defs { + if def == nil || seen[def.Entity.String()] { + continue + } + seen[def.Entity.String()] = true + module, err := findModule(ctx, def.Entity.Module) + if err != nil { + continue + } + dm, err := ctx.Backend.GetDomainModel(module.ID) + if err != nil { + continue + } + for _, e := range dm.Entities { + if e.Name == def.Entity.Name && !e.Persistable { + out = append(out, def.Entity.String()) + break + } + } + } + return out +} + +// pluralIsAre picks the verb for a list of n names. +func pluralIsAre(n int) string { + if n == 1 { + return "is" + } + return "are" +} + +// publishAssociationsFor picks the PublishAssociations value to store. +// +// false means "expose associations as an associated object id", and Mendix then +// requires the system ID attribute to be published as the entity key — so a +// service whose key is an ordinary attribute (what MDL's `expose (Attr (KEY))` +// writes) fails the build with CE7375, and a non-persistable entity cannot +// satisfy it at all because publishing its ID is forbidden. Verified on 11.12.1: +// the identical service builds 0 errors with true and CE7375 with false, for a +// persistent entity with a unique key. +// +// Defaulting to true therefore does not pick a preference; it picks the value +// that can build from the MDL people actually write. An explicit +// `PublishAssociations: false` is still honoured — that author has published an +// ID key, or wants to know. (mxcli-formula1 findings #10.4.) +func publishAssociationsFor(stmt *ast.CreateODataServiceStmt) bool { + if !stmt.PublishAssociationsSet { + return true + } + return stmt.PublishAssociations +} diff --git a/mdl/executor/cmd_odata_publish_associations_test.go b/mdl/executor/cmd_odata_publish_associations_test.go new file mode 100644 index 000000000..e8a05da2b --- /dev/null +++ b/mdl/executor/cmd_odata_publish_associations_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// mxcli-formula1 findings #10.4, and wider than reported: PublishAssociations +// false means "expose associations as an associated object id", and Mendix then +// requires the system ID attribute to be published as the entity key. MDL's +// `expose (Attr (KEY))` publishes an ordinary attribute, so the default of false +// failed the build with CE7375 for EVERY published service — persistent +// entities included, not only the non-persistable case that surfaced it. +// +// Verified on 11.12.1: the identical service (persistent entity, unique key) +// builds 0 errors with true and CE7375 with false. +func TestCreateODataService_DefaultsPublishAssociationsToTrue(t *testing.T) { + for _, persistable := range []bool{true, false} { + ctx, created, _ := publishCtx(t, "Row", persistable) + if err := createODataService(ctx, publishStmt("Row")); err != nil { + t.Fatal(err) + } + if !(*created).PublishAssociations { + t.Errorf("persistable=%v: expected PublishAssociations to default to true", persistable) + } + } +} + +// An explicit false is the author's choice — they have published an ID key, or +// they want to see what Mendix says. It is written as given. +func TestCreateODataService_ExplicitPublishAssociationsFalseIsHonoured(t *testing.T) { + ctx, created, _ := publishCtx(t, "Row", true) + stmt := publishStmt("Row") + stmt.PublishAssociations = false + stmt.PublishAssociationsSet = true + if err := createODataService(ctx, stmt); err != nil { + t.Fatal(err) + } + if (*created).PublishAssociations { + t.Error("an explicit false must not be overridden by the default") + } +} + +// An explicit false over a non-persistable entity can never build, whatever the +// key is — publishing the ID of a non-persistable entity is forbidden. Warn +// rather than let CE7375 be the first anyone hears of it. +func TestCreateODataService_WarnsOnFalseWithNonPersistable(t *testing.T) { + ctx, _, buf := publishCtx(t, "Row", false) + stmt := publishStmt("Row") + stmt.PublishAssociations = false + stmt.PublishAssociationsSet = true + if err := createODataService(ctx, stmt); err != nil { + t.Fatal(err) + } + out := buf.String() + for _, want := range []string{"PublishAssociations", "Probe.Row", "non-persistable", "CE7375"} { + if !strings.Contains(out, want) { + t.Errorf("warning should mention %q, got: %s", want, out) + } + } +} + +// The warning is for the unbuildable combination only: a persistent entity with +// an explicit false is a legitimate choice and must stay quiet. +func TestCreateODataService_NoWarningForPersistentWithFalse(t *testing.T) { + ctx, _, buf := publishCtx(t, "Row", true) + stmt := publishStmt("Row") + stmt.PublishAssociations = false + stmt.PublishAssociationsSet = true + if err := createODataService(ctx, stmt); err != nil { + t.Fatal(err) + } + if strings.Contains(buf.String(), "Warning") { + t.Errorf("no warning expected for a persistent entity, got: %s", buf.String()) + } +} diff --git a/mdl/executor/cmd_odata_service_name_test.go b/mdl/executor/cmd_odata_service_name_test.go new file mode 100644 index 000000000..1dfe52ae1 --- /dev/null +++ b/mdl/executor/cmd_odata_service_name_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "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" +) + +// publishCtx wires a module with one entity of the given persistability, and +// returns a context plus a pointer to whatever CreatePublishedODataService is +// handed. +func publishCtx(t *testing.T, entityName string, persistable bool) (*ExecContext, **model.PublishedODataService, *bytes.Buffer) { + t.Helper() + mod := mkModule("Probe") + h := mkHierarchy(mod) + dm := &domainmodel.DomainModel{ + BaseElement: model.BaseElement{ID: nextID("dm")}, + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{ + { + BaseElement: model.BaseElement{ID: nextID("ent")}, + Name: entityName, + Persistable: persistable, + Attributes: []*domainmodel.Attribute{ + {BaseElement: model.BaseElement{ID: nextID("attr")}, Name: "RowKey", Type: &domainmodel.StringAttributeType{}}, + }, + }, + }, + } + + var created *model.PublishedODataService + 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 }, + ListPublishedODataServicesFunc: func() ([]*model.PublishedODataService, error) { + return nil, nil + }, + CreatePublishedODataServiceFunc: func(svc *model.PublishedODataService) error { + created = svc + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + return ctx, &created, buf +} + +func publishStmt(entityName string) *ast.CreateODataServiceStmt { + return &ast.CreateODataServiceStmt{ + Name: ast.QualifiedName{Module: "Probe", Name: "ProbeApi"}, + Path: "odata/probe/", + Version: "1.0.0", + ODataVersion: "OData4", + Namespace: "Probe.Api", + Entities: []*ast.PublishedEntityDef{ + { + Entity: ast.QualifiedName{Module: "Probe", Name: entityName}, + ExposedName: "Rows", + ReadMode: "MICROFLOW Probe.Read_Rows", + }, + }, + } +} + +// mxcli-formula1 findings #10.1: Name (the document) and ServiceName (the name +// in the OData metadata document) are different properties, and CREATE only set +// the first — so every published service created purely from MDL failed the +// build with CE0729 "The service name should not be empty", which `mxcli check` +// cannot see. The CONSUMED path has defaulted this for CE0339 all along. +func TestCreateODataService_DefaultsServiceNameToDocumentName(t *testing.T) { + ctx, created, _ := publishCtx(t, "Row", true) + if err := createODataService(ctx, publishStmt("Row")); err != nil { + t.Fatal(err) + } + if *created == nil { + t.Fatal("expected the service to be created") + } + if got := (*created).ServiceName; got != "ProbeApi" { + t.Errorf("ServiceName = %q, want the document name %q", got, "ProbeApi") + } +} + +// An explicit ServiceName still wins — the default fills a gap, it does not +// override the author. +func TestCreateODataService_ExplicitServiceNameWins(t *testing.T) { + ctx, created, _ := publishCtx(t, "Row", true) + stmt := publishStmt("Row") + stmt.ServiceName = "PublicName" + if err := createODataService(ctx, stmt); err != nil { + t.Fatal(err) + } + if got := (*created).ServiceName; got != "PublicName" { + t.Errorf("ServiceName = %q, want %q", got, "PublicName") + } +} diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index 6abb7f27d..d58d30ff9 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -119,6 +119,7 @@ func (b *Builder) ExitCreateODataServiceStatement(ctx *parser.CreateODataService stmt.Description = value case "publishassociations": stmt.PublishAssociations = strings.EqualFold(value, "true") || strings.EqualFold(value, "yes") + stmt.PublishAssociationsSet = true case "folder": stmt.Folder = value } From 6b5db79079f8764af36a3a4f25b1e7922c79769f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:48:22 +0000 Subject: [PATCH 02/10] feat(check): report OData property names that are silently discarded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grammar accepts any `name: value` pair in an OData property list and the visitor's switch had no default, so `ReadMicroflow:` or `ServiceNam:` parsed cleanly, checked cleanly, executed "successfully" — and left the model without the property. The ALTER path has always answered "unknown OData service property"; CREATE, PUBLISH ENTITY, the OData client and the external entity had nothing. The name is lost in the visitor, so the visitor is where it has to be recorded: each switch grows a default that appends to UnknownProperties, and a check-time validator reports them as MDL-ODATA01 before anything is written. The message guesses the intended property (prefix/substring, then one edit) rather than only listing the known ones — with a list alone the reader is still diffing two spellings by eye. One correction to the report: `Pagesize:` is not among the casualties. The visitor lowercases before matching, so casing is never a typo. A test pins that, so the rule cannot start flagging it later. No false positives across every script in mdl-examples. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + cmd/mxcli/cmd_check.go | 5 + mdl/ast/ast_odata.go | 15 ++ mdl/executor/validate_odata_properties.go | 142 ++++++++++++++++++ .../validate_odata_properties_test.go | 137 +++++++++++++++++ mdl/visitor/visitor_odata.go | 8 + 6 files changed, 308 insertions(+) create mode 100644 mdl/executor/validate_odata_properties.go create mode 100644 mdl/executor/validate_odata_properties_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 25eff01f9..66c45cc11 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -392,3 +392,4 @@ extracting `OffsetExpression`/`LimitExpression`. | `mxcli check` reports `✓ Syntax OK` / `Check passed!`, then `mxcli exec` on the same script fails partway through with `failed to resolve page: page not found: Module.Page` — a button targeting a page the script creates further down. `exec` is not transactional, so the statements before the failure are already written to the .mpr | Page references are resolved in statement order at exec time. `check --references` already had an ordered pass (`validateForwardPageRefs`), but it needs `-p`; plain `check` had nothing, and plain `check` is what gets run | `mdl/executor/validate_page_order.go` (`ValidateScriptPageOrder`, MDL-PAGE01), wired in `cmd/mxcli/cmd_check.go` beside the other project-free validators | The soundness argument is what makes this work without a project: a **plain** CREATE later in the script would fail if the page already existed, so the script itself asserts the page does not exist yet and the earlier reference cannot resolve against the project either. `CREATE OR MODIFY`/`OR REPLACE` assert nothing, so they stay with `--references`, which can look. **Generalisable**: an ordering rule that seems to need project state often does not, once you find the statement that already asserts what you were going to look up. Two things the diagnostic must say and does: a cycle cannot be fixed by ordering (create one page without the linking widget, add it with `ALTER PAGE … INSERT`), and commit before executing a large script. Verified against all `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_page_order_test.go`, example `mdl-examples/bug-tests/todo-9-forward-page-reference.fail.mdl`. mxcli-todo #9 | | A sidebar navigation label renders truncated — "All tasks" shows as "All task" — under every theme (`signal`, `ledger`, `console`) and both variants, so switching theme does not help. Measured on the live DOM as `scrollWidth=56` inside `clientWidth=48` | Atlas's **closed** sidebar is an icon rail: `--navsidebar-width-closed: 48px` in Atlas's own `themesource/atlas_core/web/themes/_theme-default.scss`. The label is wider than the rail, and the closed rail expects an *icon*, not text. No mxcli theme sets any navigation width — the themes map colours — which is exactly why every theme reproduces it | `.claude/skills/mendix/theme-styling.md` (documented; no code change) | **A fix was written, verified, and reverted** — record the reason: adding `text-overflow: ellipsis` to the nav item helps only where Atlas also sets `white-space: nowrap`; elsewhere the label wraps to two readable lines, and the rule turns `All / tasks` into `All / t…`. Screenshotted both ways against the real compiled CSS. The answer belongs to the app (give nav items icons — what the rail is for — or keep the sidebar open), not to a theme that would impose it on every app. **Generalisable**: when a reported symptom traces to an upstream layout constant, reproduce the geometry against the real compiled CSS (48px container, real class names, Playwright measurement) — it takes minutes, tells you whose constant it is, and shows when the "obvious" CSS fix is a regression. mxcli-todo #19d | | A published OData service created purely from MDL passes `mxcli check` and then fails the build — `[CE0729] "The service name should not be empty."` and `[CE7375] "Attribute ID for entity 'X' must be published and be the key when associations are exposed as an associated object id."` — the second firing even with no associations exposed at all | Two defaults `CREATE ODATA SERVICE` never set. (1) `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties and only the first was set; the CONSUMED path had defaulted this for CE0339 all along. (2) `PublishAssociations` defaults to false = "associations as an associated object id", which Mendix only allows when the system `ID` is published as the key — but MDL's `expose (Attr (KEY))` publishes an ordinary attribute | `mdl/executor/cmd_odata.go` (`serviceName` fallback + heal on create-or-modify; `publishAssociationsFor`; `nonPersistablePublishedEntities` warning), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`PublishAssociationsSet`) | **Wider than reported**: the finding framed CE7375 as a non-persistable-entity problem. Measured on 11.12.1, the identical service with a PERSISTENT entity and a unique key builds 0 errors with `true` and CE7375 with `false` — so the default broke *every* published service, and non-persistable was just where it could not be worked around. Defaulting to true does not pick a preference; it picks the only value that can build from the MDL people write. Needs tri-state (`PublishAssociationsSet`) so an explicit `false` is still honoured, and `create or modify` no longer flips a stored value the author did not mention. **Why nothing caught it**: `mdl-examples/doctype-tests/10-odata-examples.mdl` sets both properties explicitly, so the repo's own example worked around both defaults. Tests `cmd_odata_service_name_test.go`, `cmd_odata_publish_associations_test.go`; examples `f1-10.1-…`, `f1-10.4-…`. mxcli-formula1 #10.1/#10.4 | +| A typo in an OData property — `ReadMicroflow:` for `ReadMode:`, `ServiceNam:` for `ServiceName:` — passes `mxcli check` and `exec` reports success, but the model does not have the property. Hours can go into wondering why a published resource ignores its read microflow | The grammar accepts any `name: value` pair inside an OData property list, and the visitor's `switch` had no `default` — so an unrecognised name was dropped between parse and AST. The ALTER path has always answered `"unknown OData service property: %s"`; CREATE, PUBLISH ENTITY, the client and the external entity had nothing | `mdl/ast/ast_odata.go` (`UnknownProperties` on four statements), `mdl/visitor/visitor_odata.go` (four `default:` arms), `mdl/executor/validate_odata_properties.go` (`ValidateODataProperties`, MDL-ODATA01), wired in `cmd/mxcli/cmd_check.go` | The visitor is where the name is lost, so the visitor is where it must be recorded — a validator over the AST alone cannot see a key that was already discarded. Carry it as `UnknownProperties` and report at check time, before anything is written. The message names the property AND guesses the intended one (prefix/substring, then one edit), because a bare known-property list still leaves the reader diffing two spellings by eye. **Correction to the report**: `Pagesize:` is *not* silently dropped — the visitor lowercases before matching, so casing is never a typo, and the test pins that. Verified against every `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_odata_properties_test.go`. mxcli-formula1 suggested issue 8 | diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 0e4d0f897..a374167db 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -173,6 +173,11 @@ Examples: // not row-scoped, so the argument is unbound (CE1571) at build time. violations = append(violations, executor.ValidatePageButtonContext(prog)...) + // Flag OData property names nothing below will act on. The grammar takes + // any `name: value` pair, so a typo used to be discarded in silence and + // the model quietly lacked what the author asked for. + violations = append(violations, executor.ValidateODataProperties(prog)...) + // Flag a page whose widgets point at a page created further down the same // script. `exec` resolves page references in statement order and is not // transactional, so this fails after earlier statements are already diff --git a/mdl/ast/ast_odata.go b/mdl/ast/ast_odata.go index 3eda46cc5..440299c24 100644 --- a/mdl/ast/ast_odata.go +++ b/mdl/ast/ast_odata.go @@ -43,6 +43,9 @@ type CreateODataClientStmt struct { // Custom HTTP headers Headers []HeaderDef + + // UnknownProperties: see CreateODataServiceStmt.UnknownProperties. + UnknownProperties []string } // HeaderDef represents a custom HTTP header entry. @@ -90,6 +93,12 @@ type CreateODataServiceStmt struct { AuthenticationTypes []string Entities []*PublishedEntityDef CreateOrModify bool // True if CREATE OR MODIFY was used + + // UnknownProperties holds property names the visitor did not recognise, in + // source order. The parser accepts any `name: value` pair, so without this + // a typo is discarded in silence and the model is quietly missing what the + // author asked for. + UnknownProperties []string } func (s *CreateODataServiceStmt) isStatement() {} @@ -105,6 +114,9 @@ type PublishedEntityDef struct { UsePaging bool PageSize int Members []*PublishedMemberDef + + // UnknownProperties: see CreateODataServiceStmt.UnknownProperties. + UnknownProperties []string } // PublishedMemberDef represents an EXPOSE member within a PUBLISH ENTITY block. @@ -150,6 +162,9 @@ type CreateExternalEntityStmt struct { Attributes []Attribute // reuse from ast_entity.go Documentation string CreateOrModify bool + + // UnknownProperties: see CreateODataServiceStmt.UnknownProperties. + UnknownProperties []string } func (s *CreateExternalEntityStmt) isStatement() {} diff --git a/mdl/executor/validate_odata_properties.go b/mdl/executor/validate_odata_properties.go new file mode 100644 index 000000000..eb666627a --- /dev/null +++ b/mdl/executor/validate_odata_properties.go @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Check-time (no-project) validation of OData property names. +// +// The grammar accepts any `name: value` pair inside an OData property list, and +// the visitor's switch had no default — so `ReadMicroflow:` or `Pagesize:` was +// discarded in silence and the model was quietly missing what the author asked +// for. The ALTER path has always answered "unknown OData service property: %s"; +// this applies the same rule to CREATE and to the PUBLISH ENTITY block. +// (mxcli-formula1 findings, suggested issue 8.) +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// Known property names, in the spelling the syntax help uses. These are for the +// error message only — the visitor is the authority on what is accepted, and it +// matches case-insensitively. +var ( + knownODataServiceProps = []string{ + "Path", "Version", "ODataVersion", "Namespace", "ServiceName", + "Summary", "Description", "PublishAssociations", "Folder", + } + knownPublishEntityProps = []string{ + "ReadMode", "InsertMode", "UpdateMode", "DeleteMode", "UsePaging", "PageSize", + } + knownODataClientProps = []string{ + "Version", "ODataVersion", "MetadataUrl", "Timeout", "ProxyType", + "Description", "ServiceUrl", "UseAuthentication", "HttpUsername", + "HttpPassword", "ClientCertificate", "ConfigurationMicroflow", + "HeadersMicroflow", "ErrorHandlingMicroflow", "ProxyHost", "ProxyPort", + "ProxyUsername", "ProxyPassword", "Folder", + } + knownExternalEntityProps = []string{ + "EntitySet", "RemoteName", "Countable", "Creatable", "Deletable", + "Updatable", "AllowCreateChangeLocally", + } +) + +// ValidateODataProperties flags (MDL-ODATA01) property names in OData +// statements that no layer below will act on. +func ValidateODataProperties(prog *ast.Program) []linter.Violation { + if prog == nil { + return nil + } + var out []linter.Violation + for _, stmt := range prog.Statements { + switch s := stmt.(type) { + case *ast.CreateODataServiceStmt: + out = append(out, unknownODataProps( + "odata service "+s.Name.String(), s.UnknownProperties, knownODataServiceProps)...) + for _, e := range s.Entities { + if e == nil { + continue + } + out = append(out, unknownODataProps( + fmt.Sprintf("publish entity %s in %s", e.Entity.String(), s.Name.String()), + e.UnknownProperties, knownPublishEntityProps)...) + } + case *ast.CreateODataClientStmt: + out = append(out, unknownODataProps( + "odata client "+s.Name.String(), s.UnknownProperties, knownODataClientProps)...) + case *ast.CreateExternalEntityStmt: + out = append(out, unknownODataProps( + "external entity "+s.Name.String(), s.UnknownProperties, knownExternalEntityProps)...) + } + } + return out +} + +func unknownODataProps(location string, unknown, known []string) []linter.Violation { + var out []linter.Violation + for _, name := range unknown { + v := linter.Violation{ + RuleID: "MDL-ODATA01", + Severity: linter.SeverityError, + Message: fmt.Sprintf("%s: unknown property %q — it is accepted by the parser and then discarded, so the model will not have it", + location, name), + Suggestion: fmt.Sprintf("Known properties here: %s.", strings.Join(known, ", ")), + } + if near := closestProperty(name, known); near != "" { + v.Suggestion = fmt.Sprintf("Did you mean %q? Known properties here: %s.", near, strings.Join(known, ", ")) + } + out = append(out, v) + } + return out +} + +// closestProperty returns the known property a misspelling most likely meant, +// or "" when nothing is close enough to be worth guessing. Case-insensitive +// prefix/substring first, then a single edit. +func closestProperty(name string, known []string) string { + lower := strings.ToLower(name) + for _, k := range known { + lk := strings.ToLower(k) + if strings.HasPrefix(lk, lower) || strings.HasPrefix(lower, lk) || strings.Contains(lk, lower) { + return k + } + } + for _, k := range known { + if withinOneEdit(lower, strings.ToLower(k)) { + return k + } + } + return "" +} + +// withinOneEdit reports whether a and b differ by at most one insertion, +// deletion or substitution. +func withinOneEdit(a, b string) bool { + if a == b { + return true + } + if len(a) > len(b) { + a, b = b, a + } + if len(b)-len(a) > 1 { + return false + } + i, j, edits := 0, 0, 0 + for i < len(a) && j < len(b) { + if a[i] == b[j] { + i++ + j++ + continue + } + edits++ + if edits > 1 { + return false + } + if len(a) == len(b) { + i++ + } + j++ + } + return true +} diff --git a/mdl/executor/validate_odata_properties_test.go b/mdl/executor/validate_odata_properties_test.go new file mode 100644 index 000000000..f033f8dbb --- /dev/null +++ b/mdl/executor/validate_odata_properties_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// mxcli-formula1 findings, suggested issue 8: the OData property switches had +// no default, so a typo was accepted by the parser, dropped by the visitor, and +// the model was quietly missing what the author asked for. The ALTER path has +// always answered "unknown OData service property". +func TestValidateODataProperties(t *testing.T) { + const prologue = "create module T;\ncreate non-persistent entity T.Row (K: string(20));\n" + + service := func(props, entityProps string) string { + return prologue + ` +create odata service T.Api ( + path: 'odata/t/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'T.Api'` + props + ` +) +authentication basic +{ + publish entity T.Row as 'Rows' ( + ReadMode: source` + entityProps + ` + ) + expose ( K as 'k' (KEY) ); +}; +` + } + + tests := []struct { + name string + script string + wantN int + wantText []string + }{ + {"clean service", service("", ""), 0, nil}, + { + "misspelt service property", + service(",\n ServiceNam: 'Api'", ""), + 1, + // The guess is the whole point: the known-property list alone still + // leaves the reader diffing two spellings by eye. + []string{"ServiceNam", "ServiceName"}, + }, + { + "unknown publish-entity property", + service("", ",\n ReadMicroflow: microflow T.Read"), + 1, + []string{"ReadMicroflow", "ReadMode"}, + }, + { + "both at once", + service(",\n Pth: 'x'", ",\n PgSize: 20"), + 2, + nil, + }, + // Property matching is case-insensitive in the visitor, so a different + // casing is NOT a typo and must not be reported. (The finding listed + // `Pagesize:` as silently dropped; it is not — it is accepted.) + {"casing is not a typo", service("", ",\n Pagesize: 20"), 0, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prog, errs := visitor.Build(tt.script) + if len(errs) > 0 { + t.Fatalf("parsing the script: %v", errs) + } + got := ValidateODataProperties(prog) + if len(got) != tt.wantN { + t.Fatalf("got %d violations, want %d: %v", len(got), tt.wantN, got) + } + for _, want := range tt.wantText { + found := false + for _, v := range got { + if strings.Contains(v.Message+v.Suggestion, want) { + found = true + } + } + if !found { + t.Errorf("expected a violation mentioning %q, got: %v", want, got) + } + } + for _, v := range got { + if v.RuleID != "MDL-ODATA01" { + t.Errorf("rule = %q, want MDL-ODATA01", v.RuleID) + } + } + }) + } +} + +func TestValidateODataProperties_ClientAndExternalEntity(t *testing.T) { + script := ` +create module T; +create odata client T.Api ( + Version: '1.0', + ODataVersion: OData4, + MetadataUrl: 'https://example.com/$metadata', + Timout: 300 +); +` + prog, errs := visitor.Build(script) + if len(errs) > 0 { + t.Fatalf("parsing the script: %v", errs) + } + got := ValidateODataProperties(prog) + if len(got) != 1 { + t.Fatalf("got %d violations, want 1: %v", len(got), got) + } + if !strings.Contains(got[0].Suggestion, "Timeout") { + t.Errorf("expected a Timeout suggestion, got: %s", got[0].Suggestion) + } +} + +func TestClosestProperty(t *testing.T) { + known := []string{"ReadMode", "InsertMode", "PageSize"} + tests := []struct{ in, want string }{ + {"ReadMod", "ReadMode"}, // one deletion + {"Readmodes", "ReadMode"}, // one insertion, different case + {"PageSiz", "PageSize"}, // prefix + {"Countable", ""}, // nothing close — no guess is better than a wrong one + {"", "ReadMode"}, // empty is a prefix of everything; harmless + } + for _, tt := range tests { + if got := closestProperty(tt.in, known); got != tt.want { + t.Errorf("closestProperty(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index d58d30ff9..3c356c114 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -70,6 +70,8 @@ func (b *Builder) ExitCreateODataClientStatement(ctx *parser.CreateODataClientSt stmt.ProxyPassword = value case "folder": stmt.Folder = value + default: + stmt.UnknownProperties = append(stmt.UnknownProperties, name) } } @@ -122,6 +124,8 @@ func (b *Builder) ExitCreateODataServiceStatement(ctx *parser.CreateODataService stmt.PublishAssociationsSet = true case "folder": stmt.Folder = value + default: + stmt.UnknownProperties = append(stmt.UnknownProperties, name) } } @@ -189,6 +193,8 @@ func (b *Builder) ExitCreateExternalEntityStatement(ctx *parser.CreateExternalEn stmt.Updatable = &boolVal case "allowcreatechangelocally", "allowcreatingandchanginglocally", "createchangelocally": stmt.AllowCreateChangeLocally = &boolVal + default: + stmt.UnknownProperties = append(stmt.UnknownProperties, name) } } @@ -356,6 +362,8 @@ func parsePublishEntityBlock(ctx parser.IPublishEntityBlockContext) *ast.Publish if n, err := strconv.Atoi(value); err == nil { entity.PageSize = n } + default: + entity.UnknownProperties = append(entity.UnknownProperties, name) } } From fa0cdb67e258569cc3c33753d3859a735ea2c01d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:57:57 +0000 Subject: [PATCH 03/10] feat(odata): let a published entity turn off Countable/Skip/Top MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three OData query options were written as literal true in the BSON writer, with no MDL above them. Countable is not a cosmetic default: it forces every read-microflow-backed resource to declare a System.ODataResponse parameter and compute a count, and over a full CSV scan that count is the expensive part of the request. They are now publish-entity properties. Tri-state is load-bearing — they default to true, so "unset" and "false" cannot share a representation or every existing script would quietly turn them off. Unset stays nil, the writer resolves nil to true, and the reader maps a stored true back to nil so DESCRIBE prints what the author wrote rather than three defaults on every resource. Verified on 11.12.1: `Countable: No` with a read microflow that takes no $Response parameter builds 0 errors — the combination that could not be expressed before. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + mdl/ast/ast_odata.go | 8 +++ mdl/backend/modelsdk/odata_read_detail.go | 18 +++++ mdl/backend/modelsdk/odata_write.go | 18 ++++- mdl/backend/modelsdk/odata_write_test.go | 17 +++++ mdl/executor/cmd_odata.go | 14 ++++ mdl/executor/cmd_odata_query_options_test.go | 73 ++++++++++++++++++++ mdl/visitor/visitor_odata.go | 14 ++++ model/types.go | 6 ++ 9 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 mdl/executor/cmd_odata_query_options_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 66c45cc11..f664ccd12 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -393,3 +393,4 @@ extracting `OffsetExpression`/`LimitExpression`. | A sidebar navigation label renders truncated — "All tasks" shows as "All task" — under every theme (`signal`, `ledger`, `console`) and both variants, so switching theme does not help. Measured on the live DOM as `scrollWidth=56` inside `clientWidth=48` | Atlas's **closed** sidebar is an icon rail: `--navsidebar-width-closed: 48px` in Atlas's own `themesource/atlas_core/web/themes/_theme-default.scss`. The label is wider than the rail, and the closed rail expects an *icon*, not text. No mxcli theme sets any navigation width — the themes map colours — which is exactly why every theme reproduces it | `.claude/skills/mendix/theme-styling.md` (documented; no code change) | **A fix was written, verified, and reverted** — record the reason: adding `text-overflow: ellipsis` to the nav item helps only where Atlas also sets `white-space: nowrap`; elsewhere the label wraps to two readable lines, and the rule turns `All / tasks` into `All / t…`. Screenshotted both ways against the real compiled CSS. The answer belongs to the app (give nav items icons — what the rail is for — or keep the sidebar open), not to a theme that would impose it on every app. **Generalisable**: when a reported symptom traces to an upstream layout constant, reproduce the geometry against the real compiled CSS (48px container, real class names, Playwright measurement) — it takes minutes, tells you whose constant it is, and shows when the "obvious" CSS fix is a regression. mxcli-todo #19d | | A published OData service created purely from MDL passes `mxcli check` and then fails the build — `[CE0729] "The service name should not be empty."` and `[CE7375] "Attribute ID for entity 'X' must be published and be the key when associations are exposed as an associated object id."` — the second firing even with no associations exposed at all | Two defaults `CREATE ODATA SERVICE` never set. (1) `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties and only the first was set; the CONSUMED path had defaulted this for CE0339 all along. (2) `PublishAssociations` defaults to false = "associations as an associated object id", which Mendix only allows when the system `ID` is published as the key — but MDL's `expose (Attr (KEY))` publishes an ordinary attribute | `mdl/executor/cmd_odata.go` (`serviceName` fallback + heal on create-or-modify; `publishAssociationsFor`; `nonPersistablePublishedEntities` warning), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`PublishAssociationsSet`) | **Wider than reported**: the finding framed CE7375 as a non-persistable-entity problem. Measured on 11.12.1, the identical service with a PERSISTENT entity and a unique key builds 0 errors with `true` and CE7375 with `false` — so the default broke *every* published service, and non-persistable was just where it could not be worked around. Defaulting to true does not pick a preference; it picks the only value that can build from the MDL people write. Needs tri-state (`PublishAssociationsSet`) so an explicit `false` is still honoured, and `create or modify` no longer flips a stored value the author did not mention. **Why nothing caught it**: `mdl-examples/doctype-tests/10-odata-examples.mdl` sets both properties explicitly, so the repo's own example worked around both defaults. Tests `cmd_odata_service_name_test.go`, `cmd_odata_publish_associations_test.go`; examples `f1-10.1-…`, `f1-10.4-…`. mxcli-formula1 #10.1/#10.4 | | A typo in an OData property — `ReadMicroflow:` for `ReadMode:`, `ServiceNam:` for `ServiceName:` — passes `mxcli check` and `exec` reports success, but the model does not have the property. Hours can go into wondering why a published resource ignores its read microflow | The grammar accepts any `name: value` pair inside an OData property list, and the visitor's `switch` had no `default` — so an unrecognised name was dropped between parse and AST. The ALTER path has always answered `"unknown OData service property: %s"`; CREATE, PUBLISH ENTITY, the client and the external entity had nothing | `mdl/ast/ast_odata.go` (`UnknownProperties` on four statements), `mdl/visitor/visitor_odata.go` (four `default:` arms), `mdl/executor/validate_odata_properties.go` (`ValidateODataProperties`, MDL-ODATA01), wired in `cmd/mxcli/cmd_check.go` | The visitor is where the name is lost, so the visitor is where it must be recorded — a validator over the AST alone cannot see a key that was already discarded. Carry it as `UnknownProperties` and report at check time, before anything is written. The message names the property AND guesses the intended one (prefix/substring, then one edit), because a bare known-property list still leaves the reader diffing two spellings by eye. **Correction to the report**: `Pagesize:` is *not* silently dropped — the visitor lowercases before matching, so casing is never a typo, and the test pins that. Verified against every `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_odata_properties_test.go`. mxcli-formula1 suggested issue 8 | +| A read-microflow-backed OData resource must declare a `System.ODataResponse` parameter and compute a count, even when the count is expensive (a full CSV scan) and nobody asked for it — with no MDL to say otherwise. Same for `$skip`/`$top` support | `Countable`, `SkipSupported` and `TopSupported` were written as literal `true` in the BSON writer's `ODataPublish$QueryOptions`; nothing above the writer could express them | `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`*bool` on `PublishedEntityDef`, `odataBoolPtr`), `model/types.go`, `mdl/executor/cmd_odata.go` (`astEntityDefToModel`), `mdl/backend/modelsdk/odata_write.go` (`boolOrDefault`), `odata_read_detail.go` (`falseOnly`) | Tri-state (`*bool`) is load-bearing: these default to **true**, so "unset" and "false" cannot share a representation or every existing script would silently turn them off. The reader maps a stored `true` back to nil (`falseOnly`) so DESCRIBE prints only what the author wrote instead of three defaults on every resource. Verified end to end on 11.12.1: `Countable: No` + a read microflow with **no** `$Response` parameter builds 0 errors, which is exactly the combination that was impossible before. Tests `cmd_odata_query_options_test.go`, `odata_write_test.go`. mxcli-formula1 #10.3 | diff --git a/mdl/ast/ast_odata.go b/mdl/ast/ast_odata.go index 440299c24..729a8a24a 100644 --- a/mdl/ast/ast_odata.go +++ b/mdl/ast/ast_odata.go @@ -115,6 +115,14 @@ type PublishedEntityDef struct { PageSize int Members []*PublishedMemberDef + // Query options. nil means "not specified" and stores Mendix's own default + // of true. Countable in particular is not free: it forces the read + // microflow to take a System.ODataResponse parameter and to compute a count + // the caller may never ask for. + Countable *bool + SkipSupported *bool + TopSupported *bool + // UnknownProperties: see CreateODataServiceStmt.UnknownProperties. UnknownProperties []string } diff --git a/mdl/backend/modelsdk/odata_read_detail.go b/mdl/backend/modelsdk/odata_read_detail.go index 953245ef3..95a5c4894 100644 --- a/mdl/backend/modelsdk/odata_read_detail.go +++ b/mdl/backend/modelsdk/odata_read_detail.go @@ -127,6 +127,13 @@ func publishedEntitySetFromRaw(raw map[string]any, byID map[string]*model.Publis UpdateMode: parseODataModeRaw(raw["UpdateMode"]), DeleteMode: parseODataModeRaw(raw["DeleteMode"]), } + // Query options round-trip so DESCRIBE can print a turned-off one. Absent + // (or true, the default) is left nil so DESCRIBE stays quiet about it. + if qo := jsToMap(raw["QueryOptions"]); qo != nil { + es.Countable = falseOnly(qo["Countable"]) + es.SkipSupported = falseOnly(qo["SkipSupported"]) + es.TopSupported = falseOnly(qo["TopSupported"]) + } if et, ok := byID[jsExtractBsonID(raw["EntityTypePointer"])]; ok { es.EntityTypeName = et.Entity } @@ -172,3 +179,14 @@ func jsExtractInt(v any) int { } return 0 } + +// falseOnly returns a pointer to false when v is present and false, else nil. +// A stored true is the default, and printing every default back would make +// DESCRIBE output noisier than what the author wrote. +func falseOnly(v any) *bool { + if v == nil || jsExtractBool(v) { + return nil + } + f := false + return &f +} diff --git a/mdl/backend/modelsdk/odata_write.go b/mdl/backend/modelsdk/odata_write.go index 34c1cc6b1..7a153da21 100644 --- a/mdl/backend/modelsdk/odata_write.go +++ b/mdl/backend/modelsdk/odata_write.go @@ -287,10 +287,14 @@ func publishedEntitySetToGen(es *model.PublishedEntitySet, entityTypeID string) addStr(g, "AlternativeExposedName", "") addBool(g, "UsePaging", es.UsePaging) addInt64(g, "PageSize", int64(es.PageSize)) + // Query options default to true (Mendix's own defaults) and are only turned + // off when the author says so. Countable is not free: it forces the read + // microflow to take a System.ODataResponse parameter and compute a count. + // (mxcli-formula1 findings #10.3.) qo := newElem("ODataPublish$QueryOptions", "") - addBool(qo, "Countable", true) - addBool(qo, "SkipSupported", true) - addBool(qo, "TopSupported", true) + addBool(qo, "Countable", boolOrDefault(es.Countable, true)) + addBool(qo, "SkipSupported", boolOrDefault(es.SkipSupported, true)) + addBool(qo, "TopSupported", boolOrDefault(es.TopSupported, true)) addPart(g, "QueryOptions", qo) if entityTypeID != "" { addIDRef(g, "EntityTypePointer", model.ID(entityTypeID)) @@ -425,3 +429,11 @@ func addByNameRefListV3(b *element.Base, name string, qnames []string) { p.Append(qn) } } + +// boolOrDefault resolves an optional bool: nil means "not specified". +func boolOrDefault(v *bool, def bool) bool { + if v == nil { + return def + } + return *v +} diff --git a/mdl/backend/modelsdk/odata_write_test.go b/mdl/backend/modelsdk/odata_write_test.go index ac86033dd..54b1bf372 100644 --- a/mdl/backend/modelsdk/odata_write_test.go +++ b/mdl/backend/modelsdk/odata_write_test.go @@ -119,6 +119,10 @@ func TestCreatePublishedODataService_RoundTrip(t *testing.T) { EntitySets: []*model.PublishedEntitySet{{ ExposedName: "Things", EntityTypeName: "MyFirstModule.Thing", ReadMode: "source", UsePaging: true, PageSize: 100, + // mxcli-formula1 #10.3: these were hardcoded true in the writer. + // An explicit false must survive the BSON round trip; SkipSupported + // is left unset here so the default still applies to it. + Countable: boolPtrLocal(false), TopSupported: boolPtrLocal(false), }}, } if err := b.CreatePublishedODataService(svc); err != nil { @@ -179,8 +183,21 @@ func TestCreatePublishedODataService_RoundTrip(t *testing.T) { if !es.UsePaging || es.PageSize != 100 { t.Errorf("entity set paging not round-tripped: %+v", es) } + if es.Countable == nil || *es.Countable { + t.Errorf("Countable=false not round-tripped: %v", es.Countable) + } + if es.TopSupported == nil || *es.TopSupported { + t.Errorf("TopSupported=false not round-tripped: %v", es.TopSupported) + } + // An unset option is stored as Mendix's default of true, and reads back as + // nil so DESCRIBE does not print a default nobody wrote. + if es.SkipSupported != nil { + t.Errorf("SkipSupported should read back nil when defaulted: %v", *es.SkipSupported) + } } +func boolPtrLocal(b bool) *bool { return &b } + // TestConsumedODataServiceToGen_ConfigMicroflowKey guards issue #728: the config // microflow must be serialized under the version-appropriate BSON key. On // 11.10+ that is ConfigurationEntityMicroflow (writing the pre-11.10 diff --git a/mdl/executor/cmd_odata.go b/mdl/executor/cmd_odata.go index ee5f241e2..737d4bda6 100644 --- a/mdl/executor/cmd_odata.go +++ b/mdl/executor/cmd_odata.go @@ -412,6 +412,17 @@ func outputPublishedODataServiceMDL(ctx *ExecContext, svc *model.PublishedODataS modeProps = append(modeProps, "UsePaging: Yes") modeProps = append(modeProps, fmt.Sprintf("PageSize: %d", es.PageSize)) } + // Only a turned-off query option is worth printing; true is the + // default and would be noise on every resource. + if es.Countable != nil && !*es.Countable { + modeProps = append(modeProps, "Countable: No") + } + if es.SkipSupported != nil && !*es.SkipSupported { + modeProps = append(modeProps, "SkipSupported: No") + } + if es.TopSupported != nil && !*es.TopSupported { + modeProps = append(modeProps, "TopSupported: No") + } if len(modeProps) > 0 { fmt.Fprintf(ctx.Output, " (\n %s\n )", strings.Join(modeProps, ",\n ")) } @@ -1751,6 +1762,9 @@ func astEntityDefToModel(ctx *ExecContext, def *ast.PublishedEntityDef) (*model. UpdateMode: def.UpdateMode, DeleteMode: def.DeleteMode, UsePaging: def.UsePaging, + Countable: def.Countable, + SkipSupported: def.SkipSupported, + TopSupported: def.TopSupported, PageSize: def.PageSize, } diff --git a/mdl/executor/cmd_odata_query_options_test.go b/mdl/executor/cmd_odata_query_options_test.go new file mode 100644 index 000000000..a7984e46d --- /dev/null +++ b/mdl/executor/cmd_odata_query_options_test.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mxcli-formula1 findings #10.3: Countable, SkipSupported and TopSupported were +// hardcoded true in the BSON writer with no MDL to turn any of them off. That is +// not a cosmetic gap — Countable forces every read-microflow-backed resource to +// declare a System.ODataResponse parameter and compute a count, which over a +// 27533-row CSV scan is not free. +func TestPublishEntityQueryOptions_ParseAndCarry(t *testing.T) { + script := ` +create module Q; +create non-persistent entity Q.Row (K: string(20)); +create odata service Q.Api ( + path: 'odata/q/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'Q.Api' +) +authentication basic +{ + publish entity Q.Row as 'Rows' ( + ReadMode: microflow Q.Read, + Countable: No, + TopSupported: No + ) + expose ( K as 'k' (KEY) ); +}; +` + prog, errs := visitor.Build(script) + if len(errs) > 0 { + t.Fatalf("parsing the script: %v", errs) + } + + var def *ast.PublishedEntityDef + for _, stmt := range prog.Statements { + if s, ok := stmt.(*ast.CreateODataServiceStmt); ok && len(s.Entities) == 1 { + def = s.Entities[0] + } + } + if def == nil { + t.Fatal("expected one published entity") + } + + if def.Countable == nil || *def.Countable { + t.Errorf("Countable = %v, want an explicit false", def.Countable) + } + if def.TopSupported == nil || *def.TopSupported { + t.Errorf("TopSupported = %v, want an explicit false", def.TopSupported) + } + // Unmentioned stays nil, which the writer turns into Mendix's default of + // true — distinguishing "unset" from "false" is the whole point. + if def.SkipSupported != nil { + t.Errorf("SkipSupported = %v, want nil for an unmentioned property", *def.SkipSupported) + } + + // And the executor carries them onto the entity set it builds. + _, es := astEntityDefToModel(nil, def) + if es.Countable == nil || *es.Countable { + t.Errorf("entity set Countable = %v, want an explicit false", es.Countable) + } + if es.SkipSupported != nil { + t.Error("entity set SkipSupported should stay unset") + } +} diff --git a/mdl/visitor/visitor_odata.go b/mdl/visitor/visitor_odata.go index 3c356c114..b45380676 100644 --- a/mdl/visitor/visitor_odata.go +++ b/mdl/visitor/visitor_odata.go @@ -362,6 +362,12 @@ func parsePublishEntityBlock(ctx parser.IPublishEntityBlockContext) *ast.Publish if n, err := strconv.Atoi(value); err == nil { entity.PageSize = n } + case "countable": + entity.Countable = odataBoolPtr(value) + case "skipsupported": + entity.SkipSupported = odataBoolPtr(value) + case "topsupported": + entity.TopSupported = odataBoolPtr(value) default: entity.UnknownProperties = append(entity.UnknownProperties, name) } @@ -440,3 +446,11 @@ func parseExposeMembers(ctx parser.IExposeClauseContext) []*ast.PublishedMemberD return members } + +// odataBoolPtr parses an OData property value as a bool, keeping "specified" +// distinct from "true": these properties default to true, so only an explicit +// value may turn one off. +func odataBoolPtr(value string) *bool { + b := strings.EqualFold(value, "true") || strings.EqualFold(value, "yes") + return &b +} diff --git a/model/types.go b/model/types.go index 4b7a89fa7..7ee133361 100644 --- a/model/types.go +++ b/model/types.go @@ -473,6 +473,12 @@ type PublishedEntitySet struct { DeleteMode string `json:"deleteMode,omitempty"` UsePaging bool `json:"usePaging,omitempty"` PageSize int `json:"pageSize,omitempty"` + + // OData query options. nil means "not specified" and is written as Mendix's + // own default of true; only an explicit false turns one off. + Countable *bool `json:"countable,omitempty"` + SkipSupported *bool `json:"skipSupported,omitempty"` + TopSupported *bool `json:"topSupported,omitempty"` } // PublishedMember represents a member (attribute/association/id) published in an OData entity type. From 04aadde579f1c6cc42d7ee5fcfb57facdfb82240 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 16:59:19 +0000 Subject: [PATCH 04/10] fix(odata): make DESCRIBE of a published service re-executable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three slips in one emit block, each of which broke the DESCRIBE-roundtrip property the review checklist asks for: - `ReadMode: CallMicroflow:Module.Read` is the backend's storage spelling and matches no MDL value. It now prints `microflow Module.Read`, the form the author wrote. - `expose (...)` takes a bare member name; the stored name is fully qualified, so `Module.Entity.Attr` was emitted and did not parse. `IsPartOfKey` becomes `KEY` for the same reason — both parse, but only one is documented. - `as ''` printed the entity TYPE's exposed name where the entity SET's belongs. That one is invisible until the two differ (Studio Pro exposes the type singular and the set plural), and then a describe -> exec cycle silently renames the set the $metadata serves. Proved by round trip rather than by eye: describe -> check parses, then drop the service, exec the describe output, describe again — byte-identical, and mxbuild reports 0 errors on the rebuilt model. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + mdl/executor/cmd_odata.go | 56 +++++++-- .../cmd_odata_describe_roundtrip_test.go | 118 ++++++++++++++++++ 3 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 mdl/executor/cmd_odata_describe_roundtrip_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index f664ccd12..f809c0e6f 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -394,3 +394,4 @@ extracting `OffsetExpression`/`LimitExpression`. | A published OData service created purely from MDL passes `mxcli check` and then fails the build — `[CE0729] "The service name should not be empty."` and `[CE7375] "Attribute ID for entity 'X' must be published and be the key when associations are exposed as an associated object id."` — the second firing even with no associations exposed at all | Two defaults `CREATE ODATA SERVICE` never set. (1) `Name` (the document) and `ServiceName` (the name in the OData metadata document) are different properties and only the first was set; the CONSUMED path had defaulted this for CE0339 all along. (2) `PublishAssociations` defaults to false = "associations as an associated object id", which Mendix only allows when the system `ID` is published as the key — but MDL's `expose (Attr (KEY))` publishes an ordinary attribute | `mdl/executor/cmd_odata.go` (`serviceName` fallback + heal on create-or-modify; `publishAssociationsFor`; `nonPersistablePublishedEntities` warning), `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`PublishAssociationsSet`) | **Wider than reported**: the finding framed CE7375 as a non-persistable-entity problem. Measured on 11.12.1, the identical service with a PERSISTENT entity and a unique key builds 0 errors with `true` and CE7375 with `false` — so the default broke *every* published service, and non-persistable was just where it could not be worked around. Defaulting to true does not pick a preference; it picks the only value that can build from the MDL people write. Needs tri-state (`PublishAssociationsSet`) so an explicit `false` is still honoured, and `create or modify` no longer flips a stored value the author did not mention. **Why nothing caught it**: `mdl-examples/doctype-tests/10-odata-examples.mdl` sets both properties explicitly, so the repo's own example worked around both defaults. Tests `cmd_odata_service_name_test.go`, `cmd_odata_publish_associations_test.go`; examples `f1-10.1-…`, `f1-10.4-…`. mxcli-formula1 #10.1/#10.4 | | A typo in an OData property — `ReadMicroflow:` for `ReadMode:`, `ServiceNam:` for `ServiceName:` — passes `mxcli check` and `exec` reports success, but the model does not have the property. Hours can go into wondering why a published resource ignores its read microflow | The grammar accepts any `name: value` pair inside an OData property list, and the visitor's `switch` had no `default` — so an unrecognised name was dropped between parse and AST. The ALTER path has always answered `"unknown OData service property: %s"`; CREATE, PUBLISH ENTITY, the client and the external entity had nothing | `mdl/ast/ast_odata.go` (`UnknownProperties` on four statements), `mdl/visitor/visitor_odata.go` (four `default:` arms), `mdl/executor/validate_odata_properties.go` (`ValidateODataProperties`, MDL-ODATA01), wired in `cmd/mxcli/cmd_check.go` | The visitor is where the name is lost, so the visitor is where it must be recorded — a validator over the AST alone cannot see a key that was already discarded. Carry it as `UnknownProperties` and report at check time, before anything is written. The message names the property AND guesses the intended one (prefix/substring, then one edit), because a bare known-property list still leaves the reader diffing two spellings by eye. **Correction to the report**: `Pagesize:` is *not* silently dropped — the visitor lowercases before matching, so casing is never a typo, and the test pins that. Verified against every `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_odata_properties_test.go`. mxcli-formula1 suggested issue 8 | | A read-microflow-backed OData resource must declare a `System.ODataResponse` parameter and compute a count, even when the count is expensive (a full CSV scan) and nobody asked for it — with no MDL to say otherwise. Same for `$skip`/`$top` support | `Countable`, `SkipSupported` and `TopSupported` were written as literal `true` in the BSON writer's `ODataPublish$QueryOptions`; nothing above the writer could express them | `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`*bool` on `PublishedEntityDef`, `odataBoolPtr`), `model/types.go`, `mdl/executor/cmd_odata.go` (`astEntityDefToModel`), `mdl/backend/modelsdk/odata_write.go` (`boolOrDefault`), `odata_read_detail.go` (`falseOnly`) | Tri-state (`*bool`) is load-bearing: these default to **true**, so "unset" and "false" cannot share a representation or every existing script would silently turn them off. The reader maps a stored `true` back to nil (`falseOnly`) so DESCRIBE prints only what the author wrote instead of three defaults on every resource. Verified end to end on 11.12.1: `Countable: No` + a read microflow with **no** `$Response` parameter builds 0 errors, which is exactly the combination that was impossible before. Tests `cmd_odata_query_options_test.go`, `odata_write_test.go`. mxcli-formula1 #10.3 | +| `describe odata service Module.Api` emits MDL that will not parse — `ReadMode: CallMicroflow:Module.Read` matches no value, `expose (Module.Entity.Attr ...)` where the clause takes a bare member name — and quietly renames the entity set, printing the entity TYPE's exposed name in the `as '…'` position where the entity SET's belongs | Three independent slips in one emit block. The backend stores a microflow-backed mode as `CallMicroflow:` and a member fully qualified; DESCRIBE printed the stored forms verbatim. The set/type name confusion is invisible in a single-word case and only shows when the two differ (Studio Pro's convention is singular type, plural set) | `mdl/executor/cmd_odata.go` (`odataModeToMDL`, `bareMemberName`, entity-set exposed name, `KEY` over `IsPartOfKey`) | Storage form ≠ input form: anywhere DESCRIBE prints a value read back from the backend, ask whether the *parser* accepts that spelling — `mx check` and the linter never see DESCRIBE output, so nothing else can catch it. Proved by round trip rather than by eye: describe → check (parses) → drop → exec the output → describe again → **byte-identical**, and the rebuilt model reports 0 errors from mxbuild. Tests `cmd_odata_describe_roundtrip_test.go`. mxcli-formula1 #10.5 | diff --git a/mdl/executor/cmd_odata.go b/mdl/executor/cmd_odata.go index 737d4bda6..28a0a1d89 100644 --- a/mdl/executor/cmd_odata.go +++ b/mdl/executor/cmd_odata.go @@ -392,21 +392,31 @@ func outputPublishedODataServiceMDL(ctx *ExecContext, svc *model.PublishedODataS es = entitySetByEntityName[et.Entity] } - // PUBLISH ENTITY line with modes - fmt.Fprintf(ctx.Output, " publish entity %s as '%s'", et.Entity, et.ExposedName) + // PUBLISH ENTITY line with modes. + // + // `AS ''` is the ENTITY SET's exposed name — that is what the + // served $metadata calls the set, and what re-executing this output + // must reproduce. Printing the entity TYPE's exposed name here + // silently renamed the set on a describe -> exec round trip + // (mxcli-formula1 findings #10.5). + exposedName := et.ExposedName + if es != nil && es.ExposedName != "" { + exposedName = es.ExposedName + } + fmt.Fprintf(ctx.Output, " publish entity %s as '%s'", et.Entity, exposedName) if es != nil { var modeProps []string if es.ReadMode != "" { - modeProps = append(modeProps, fmt.Sprintf("ReadMode: %s", es.ReadMode)) + modeProps = append(modeProps, fmt.Sprintf("ReadMode: %s", odataModeToMDL(es.ReadMode))) } if es.InsertMode != "" { - modeProps = append(modeProps, fmt.Sprintf("InsertMode: %s", es.InsertMode)) + modeProps = append(modeProps, fmt.Sprintf("InsertMode: %s", odataModeToMDL(es.InsertMode))) } if es.UpdateMode != "" { - modeProps = append(modeProps, fmt.Sprintf("UpdateMode: %s", es.UpdateMode)) + modeProps = append(modeProps, fmt.Sprintf("UpdateMode: %s", odataModeToMDL(es.UpdateMode))) } if es.DeleteMode != "" { - modeProps = append(modeProps, fmt.Sprintf("DeleteMode: %s", es.DeleteMode)) + modeProps = append(modeProps, fmt.Sprintf("DeleteMode: %s", odataModeToMDL(es.DeleteMode))) } if es.UsePaging { modeProps = append(modeProps, "UsePaging: Yes") @@ -441,10 +451,17 @@ func outputPublishedODataServiceMDL(ctx *ExecContext, svc *model.PublishedODataS modifiers = append(modifiers, "Sortable") } if m.IsPartOfKey { - modifiers = append(modifiers, "IsPartOfKey") + // KEY is the spelling the syntax help documents; + // IsPartOfKey parses too, but only one belongs in + // output meant to be re-executed. + modifiers = append(modifiers, "KEY") } - line := fmt.Sprintf(" %s as '%s'", m.Name, m.ExposedName) + // The member is stored fully qualified + // (Module.Entity.Member), while `expose (...)` takes a bare + // member name — so emitting the stored form produced MDL + // that does not parse (mxcli-formula1 findings #10.5). + line := fmt.Sprintf(" %s as '%s'", bareMemberName(m.Name), m.ExposedName) if len(modifiers) > 0 { line += fmt.Sprintf(" (%s)", strings.Join(modifiers, ", ")) } @@ -1886,3 +1903,26 @@ func publishAssociationsFor(stmt *ast.CreateODataServiceStmt) bool { } return stmt.PublishAssociations } + +// odataModeToMDL turns a stored Read/Change mode into the MDL spelling that +// parses back. The backend stores a microflow-backed mode as +// "CallMicroflow:Module.Name" (and accepts "MICROFLOW Module.Name" on the way +// in), but a bare `CallMicroflow:Qualified.Name` matches no MDL value — so +// DESCRIBE was emitting something it could not read (mxcli-formula1 #10.5). +func odataModeToMDL(mode string) string { + for _, prefix := range []string{"CallMicroflow:", "MICROFLOW ", "microflow "} { + if rest := strings.TrimPrefix(mode, prefix); rest != mode { + return "microflow " + strings.TrimSpace(rest) + } + } + return mode +} + +// bareMemberName strips a Module.Entity. prefix from a published member name, +// leaving the member name `expose (...)` accepts. +func bareMemberName(name string) string { + if i := strings.LastIndex(name, "."); i >= 0 { + return name[i+1:] + } + return name +} diff --git a/mdl/executor/cmd_odata_describe_roundtrip_test.go b/mdl/executor/cmd_odata_describe_roundtrip_test.go new file mode 100644 index 000000000..f536fe4f4 --- /dev/null +++ b/mdl/executor/cmd_odata_describe_roundtrip_test.go @@ -0,0 +1,118 @@ +// 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" +) + +// mxcli-formula1 findings #10.5: DESCRIBE emitted a form it could not parse. +// The project's review checklist asks DESCRIBE to produce re-executable MDL, so +// three separate slips each broke that: a stored mode spelling with no MDL +// equivalent, the entity TYPE's exposed name where the entity SET's belongs +// (silently renaming the set on a re-exec), and fully-qualified member names in +// an `expose (...)` clause that takes bare ones. +func TestDescribeODataService_EmitsReExecutableMDL(t *testing.T) { + mod := mkModule("F1") + svc := &model.PublishedODataService{ + BaseElement: model.BaseElement{ID: nextID("pos")}, + ContainerID: mod.ID, + Name: "LapApi", + ServiceName: "LapApi", + Path: "odata/f1/", + Version: "1.0.0", + ODataVersion: "OData4", + Namespace: "F1.Laps", + EntityTypes: []*model.PublishedEntityType{{ + Entity: "F1.Lap", + // The TYPE is exposed singular, the SET plural — Studio Pro's own + // convention, and the reason printing the wrong one is invisible + // until someone re-executes the output. + ExposedName: "Lap", + Members: []*model.PublishedMember{ + {Kind: "attribute", Name: "F1.Lap.LapKey", ExposedName: "lapKey", IsPartOfKey: true}, + {Kind: "attribute", Name: "F1.Lap.Driver", ExposedName: "driver", Filterable: true}, + }, + }}, + EntitySets: []*model.PublishedEntitySet{{ + ExposedName: "Laps", + EntityTypeName: "F1.Lap", + ReadMode: "CallMicroflow:F1.Read_Laps", + InsertMode: "NotSupported", + Countable: boolPtr(false), + }}, + } + h := mkHierarchy(mod) + withContainer(h, svc.ContainerID, mod.ID) + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListPublishedODataServicesFunc: func() ([]*model.PublishedODataService, error) { + return []*model.PublishedODataService{svc}, nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + assertNoError(t, describeODataService(ctx, ast.QualifiedName{Module: "F1", Name: "LapApi"})) + out := buf.String() + + for _, want := range []string{ + "publish entity F1.Lap as 'Laps'", // the SET name, not the TYPE name + "ReadMode: microflow F1.Read_Laps", // not "CallMicroflow:F1.Read_Laps" + "LapKey as 'lapKey' (KEY)", // bare member, documented modifier + "driver", // the second member survives + "Countable: No", // a turned-off option is printed + } { + if !strings.Contains(out, want) { + t.Errorf("describe output should contain %q, got:\n%s", want, out) + } + } + + for _, unwanted := range []string{ + "CallMicroflow:", // parses as nothing + "F1.Lap.LapKey", // qualified member in an expose clause + "as 'Lap'", // the entity type name in the AS position + "IsPartOfKey", // parses, but KEY is the documented spelling + "SkipSupported", // unset options stay unprinted + "TopSupported", + } { + if strings.Contains(out, unwanted) { + t.Errorf("describe output should not contain %q, got:\n%s", unwanted, out) + } + } +} + +func TestODataModeToMDL(t *testing.T) { + tests := []struct{ in, want string }{ + {"CallMicroflow:M.Read", "microflow M.Read"}, + {"MICROFLOW M.Read", "microflow M.Read"}, + {"microflow M.Read", "microflow M.Read"}, + // Everything else is already an MDL spelling and must pass through. + {"source", "source"}, + {"NotSupported", "NotSupported"}, + {"", ""}, + } + for _, tt := range tests { + if got := odataModeToMDL(tt.in); got != tt.want { + t.Errorf("odataModeToMDL(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} + +func TestBareMemberName(t *testing.T) { + tests := []struct{ in, want string }{ + {"F1.Lap.LapKey", "LapKey"}, + {"Lap.LapKey", "LapKey"}, + {"LapKey", "LapKey"}, + {"", ""}, + } + for _, tt := range tests { + if got := bareMemberName(tt.in); got != tt.want { + t.Errorf("bareMemberName(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} From 305a9fa583c87664b1bd60341b0a54c5e676d18c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:01:28 +0000 Subject: [PATCH 05/10] docs(odata): document ReadMode microflow, ServiceName and the query options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ReadMode: microflow Module.MF` has worked through the whole pipeline — grammar, visitor, AST, BSON — and was documented nowhere, so the only way to find it was to read the Go source. Without it, "publish persistent entities" is the only shape the docs describe, which sends anyone with data outside Mendix down a materialise-into-the-database path they do not need. The syntax topic now carries ReadMode/InsertMode/UpdateMode/DeleteMode in their microflow form, ServiceName and PublishAssociations with their defaults, and the three query options. The skill gains a worked non-persistable example — read microflow, no copy of the data, no refresh job — with the two things that bite first: the $Response parameter Countable requires, and why PublishAssociations must stay at its default there. The skill's existing example also carried `PublishAssociations: No`, which cannot build: object-id mode requires the system ID as the published key, and that example publishes an ordinary attribute. Removed, with the reason. The new example was executed against a real .mpr and builds 0 errors on 11.12.1. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/mendix/odata-data-sharing.md | 66 ++++++++++++++++++++- cmd/mxcli/syntax/features_integration.go | 33 ++++++++--- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/.claude/skills/mendix/odata-data-sharing.md b/.claude/skills/mendix/odata-data-sharing.md index acefc6a29..a61793172 100644 --- a/.claude/skills/mendix/odata-data-sharing.md +++ b/.claude/skills/mendix/odata-data-sharing.md @@ -223,8 +223,12 @@ create odata service ProductApi.ProductDataApi ( ODataVersion: OData4, namespace: 'DefaultNamespace', ServiceName: 'ProductDataApi', - Summary: 'Product and customer data API', - PublishAssociations: No + Summary: 'Product and customer data API' + -- PublishAssociations is left at its default (Yes = associations as links). + -- Setting it to No means "associations as an associated object id", which + -- Mendix only allows when the system ID is published as the key — publishing + -- an ordinary attribute as the key then fails the build with CE7375, even + -- when no associations are exposed at all. ) authentication basic { @@ -412,6 +416,64 @@ alter entity ShopClient.Product set allow_create_change_locally = true; alter entity ShopClient.Product set allow_create_change_locally = false; ``` +## Publishing a Non-Persistable Entity (no copy of the data) + +A published entity does **not** have to be persistable. Back it with a read +microflow and the rows are produced per request — nothing is stored, and there +is no refresh job to keep a copy in step with the source. This is the shape to +use when the data lives outside Mendix (an external database, a CSV, an API). + +```sql +create non-persistent entity Api.Lap ( + LapKey: string(60), + Driver: string(120), + LapTime: decimal +); + +-- While Countable is Yes (the default), the read microflow MUST take a +-- $Response: System.ODataResponse parameter — Mendix asks it for the count. +CREATE MICROFLOW Api.Read_Laps ($Response: System.ODataResponse) + RETURNS List of Api.Lap AS $Laps +BEGIN + -- retrieve from wherever the data actually lives, e.g. EXECUTE DATABASE QUERY + $Laps = CREATE LIST OF Api.Lap; + RETURN $Laps; +END; + +create odata service Api.LapApi ( + path: 'odata/laps/', + version: '1.0.0', + ODataVersion: OData4, + namespace: 'Api.Laps' +) +authentication basic +{ + publish entity Api.Lap as 'Laps' ( + ReadMode: microflow Api.Read_Laps, + InsertMode: not_supported, + UpdateMode: not_supported, + DeleteMode: not_supported + ) + expose ( + LapKey as 'lapKey' (KEY, Filterable, Sortable), + Driver (Filterable, Sortable), + LapTime (Sortable) + ); +}; +``` + +Two things worth knowing before you write this: + +- **`ReadMode: microflow Module.MF`** is the whole feature. `InsertMode`, + `UpdateMode` and `DeleteMode` take the same form for a read-write resource. +- **Counting is not free.** If the count means a full scan of the underlying + source, set `Countable: No` on the published entity — the read microflow then + takes no parameters at all. `SkipSupported: No` and `TopSupported: No` turn + off `$skip` and `$top` the same way. All three default to Yes. + +`PublishAssociations` must stay at its default (Yes) here: a non-persistable +entity cannot publish its ID, so object-id mode can never build for it. + ## Step-by-Step: Read-Write API with Microflow Handlers For write operations (insert, update, delete), the OData service delegates to microflows that map between the view entity and the underlying persistent entities. diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 764b8d465..506a6e8a2 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -71,23 +71,30 @@ func init() { Keywords: []string{ "create odata service", "publish entity", "publish odata", "expose", "key", "navigation property", "association exposure", - "authentication", "page size", + "authentication", "page size", "servicename", "publishassociations", + "readmode microflow", "non-persistable", "countable", "skipsupported", + "topsupported", }, Syntax: "CREATE [OR MODIFY] ODATA SERVICE Module.Name (\n" + " path: 'odata/customers/', -- no leading slash; trailing slash required\n" + " version: '1.0.0',\n" + " ODataVersion: OData4,\n" + - " namespace: 'Module.Customers'\n" + + " namespace: 'Module.Customers',\n" + + " ServiceName: 'CustomerApi', -- optional; defaults to the document name\n" + + " PublishAssociations: Yes -- optional; default Yes (associations as links)\n" + ")\n" + "authentication basic, session\n" + "{\n" + " publish entity Module.Entity as 'EntitySet' (\n" + - " ReadMode: source,\n" + - " InsertMode: source | not_supported,\n" + - " UpdateMode: source | not_supported,\n" + - " DeleteMode: source | not_supported,\n" + + " ReadMode: source | microflow Module.Read_X,\n" + + " InsertMode: source | not_supported | microflow Module.Insert_X,\n" + + " UpdateMode: source | not_supported | microflow Module.Update_X,\n" + + " DeleteMode: source | not_supported | microflow Module.Delete_X,\n" + " UsePaging: Yes,\n" + - " PageSize: 100\n" + + " PageSize: 100,\n" + + " Countable: No, -- default Yes; No drops the $Response requirement\n" + + " SkipSupported: No, -- default Yes ($skip)\n" + + " TopSupported: No -- default Yes ($top)\n" + " )\n" + " expose (\n" + " KeyAttr as 'ExposedKey' (KEY, Filterable, Sortable),\n" + @@ -96,7 +103,15 @@ func init() { " );\n" + "};\n" + "\n" + - "GRANT ACCESS ON ODATA SERVICE Module.Name TO Module.Role;", + "GRANT ACCESS ON ODATA SERVICE Module.Name TO Module.Role;\n" + + "\n" + + "-- A NON-PERSISTABLE entity can be published: back it with a read\n" + + "-- microflow returning a list of that entity. Nothing is stored, so\n" + + "-- there is no copy of the data in the database.\n" + + "--\n" + + "-- While Countable is Yes (the default) the read microflow must take a\n" + + "-- $Response: System.ODataResponse parameter and set its Count; with\n" + + "-- Countable: No it takes no parameters at all.", Example: "create persistent entity Shop.Customer (\n" + " Email: string(200) unique error 'unique' required error 'required',\n" + " Name: string(200)\n" + @@ -156,7 +171,7 @@ func init() { "body", "response", "mapping", "authentication", "json structure", "import mapping", "export mapping", }, - Syntax: "CREATE [OR MODIFY] REST CLIENT Module.Name (\n BaseUrl: 'https://...',\n Authentication: NONE | BASIC (...)\n)\n{\n OPERATION Name {\n Method: GET|POST|PUT|DELETE|PATCH,\n Path: '/path/{param}',\n Parameters: ($param: Type),\n Query: ($param: Type),\n Headers: ('Key' = 'Value'),\n Timeout: 30,\n Body: JSON FROM $var | MAPPING Entity { jsonField = Attribute, ... },\n Response: JSON AS $var | MAPPING Entity { Attribute = jsonField, ... }\n }\n};\n\n-- MAPPING takes a target ENTITY plus a body listing the JSON fields; Mendix\n-- stores it inline on the operation. An existing import/export mapping\n-- document cannot be referenced here (rejected as MDL-REST01).", + Syntax: "CREATE [OR MODIFY] REST CLIENT Module.Name (\n BaseUrl: 'https://...',\n Authentication: NONE | BASIC (...)\n)\n{\n OPERATION Name {\n Method: GET|POST|PUT|DELETE|PATCH,\n Path: '/path/{param}',\n Parameters: ($param: Type),\n Query: ($param: Type),\n Headers: ('Key' = 'Value'),\n Timeout: 30,\n Body: JSON FROM $var | MAPPING Entity { jsonField = Attribute, ... },\n Response: JSON AS $var | MAPPING Entity { Attribute = jsonField, ... }\n }\n};\n\n-- MAPPING takes a target ENTITY plus a body listing the JSON fields; Mendix\n-- stores it inline on the operation. An existing import/export mapping\n-- document cannot be referenced here (rejected as MDL-REST01).", Example: "CREATE REST CLIENT Module.PetStore (\n BaseUrl: 'https://petstore.example.com/api',\n Authentication: NONE\n)\n{\n OPERATION GetPet {\n Method: GET,\n Path: '/pets/{id}',\n Parameters: ($id: String),\n Query: ($verbose: String),\n Response: MAPPING Module.Pet {\n Name = name,\n Status = status\n }\n }\n};", SeeAlso: []string{"rest", "rest.published"}, }) From 1fb51d7fcfba1de84eb1a19e864180be5fb81539 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:06:34 +0000 Subject: [PATCH 06/10] fix(check): warn on a database-connection type Studio Pro does not offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill's table listed `Redshift` and `SQLServer`. Neither is in Studio Pro's connector picker on any version available here — read out of the shipped bundle at modeler/ide-client/database-connector-editor/, identical on 11.10.0, 11.12.1 and (per the report) 11.13.0: MSSQL, MySQL, Oracle, PostgreSQL, Snowflake, BYOD. mxcli writes the type string through unchanged and mxbuild does not validate it either — `type 'Redshift'` builds 0 errors — so a wrong value hides behind a green build and only shows up as a connection that does not connect. `check` now says so. A warning rather than an error: the set is version-specific and mxcli cannot prove a string wrong on a version it has not seen. The table was also missing the entry that matters most. `BYOD` ("Other") forces connection-string configuration and skips the driver-presence check, which makes any JDBC driver Mendix ships no picker entry for usable by dropping the JAR in userlib/. That is what the finding needed for DuckDB, and it read as impossible. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .claude/skills/mendix/database-connections.md | 33 ++++++-- cmd/mxcli/cmd_check.go | 5 ++ mdl/executor/validate_database_type.go | 78 +++++++++++++++++ mdl/executor/validate_database_type_test.go | 83 +++++++++++++++++++ 5 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 mdl/executor/validate_database_type.go create mode 100644 mdl/executor/validate_database_type_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index f809c0e6f..a3ec58155 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -395,3 +395,4 @@ extracting `OffsetExpression`/`LimitExpression`. | A typo in an OData property — `ReadMicroflow:` for `ReadMode:`, `ServiceNam:` for `ServiceName:` — passes `mxcli check` and `exec` reports success, but the model does not have the property. Hours can go into wondering why a published resource ignores its read microflow | The grammar accepts any `name: value` pair inside an OData property list, and the visitor's `switch` had no `default` — so an unrecognised name was dropped between parse and AST. The ALTER path has always answered `"unknown OData service property: %s"`; CREATE, PUBLISH ENTITY, the client and the external entity had nothing | `mdl/ast/ast_odata.go` (`UnknownProperties` on four statements), `mdl/visitor/visitor_odata.go` (four `default:` arms), `mdl/executor/validate_odata_properties.go` (`ValidateODataProperties`, MDL-ODATA01), wired in `cmd/mxcli/cmd_check.go` | The visitor is where the name is lost, so the visitor is where it must be recorded — a validator over the AST alone cannot see a key that was already discarded. Carry it as `UnknownProperties` and report at check time, before anything is written. The message names the property AND guesses the intended one (prefix/substring, then one edit), because a bare known-property list still leaves the reader diffing two spellings by eye. **Correction to the report**: `Pagesize:` is *not* silently dropped — the visitor lowercases before matching, so casing is never a typo, and the test pins that. Verified against every `mdl-examples/**/*.mdl` for false positives (0). Tests `mdl/executor/validate_odata_properties_test.go`. mxcli-formula1 suggested issue 8 | | A read-microflow-backed OData resource must declare a `System.ODataResponse` parameter and compute a count, even when the count is expensive (a full CSV scan) and nobody asked for it — with no MDL to say otherwise. Same for `$skip`/`$top` support | `Countable`, `SkipSupported` and `TopSupported` were written as literal `true` in the BSON writer's `ODataPublish$QueryOptions`; nothing above the writer could express them | `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`*bool` on `PublishedEntityDef`, `odataBoolPtr`), `model/types.go`, `mdl/executor/cmd_odata.go` (`astEntityDefToModel`), `mdl/backend/modelsdk/odata_write.go` (`boolOrDefault`), `odata_read_detail.go` (`falseOnly`) | Tri-state (`*bool`) is load-bearing: these default to **true**, so "unset" and "false" cannot share a representation or every existing script would silently turn them off. The reader maps a stored `true` back to nil (`falseOnly`) so DESCRIBE prints only what the author wrote instead of three defaults on every resource. Verified end to end on 11.12.1: `Countable: No` + a read microflow with **no** `$Response` parameter builds 0 errors, which is exactly the combination that was impossible before. Tests `cmd_odata_query_options_test.go`, `odata_write_test.go`. mxcli-formula1 #10.3 | | `describe odata service Module.Api` emits MDL that will not parse — `ReadMode: CallMicroflow:Module.Read` matches no value, `expose (Module.Entity.Attr ...)` where the clause takes a bare member name — and quietly renames the entity set, printing the entity TYPE's exposed name in the `as '…'` position where the entity SET's belongs | Three independent slips in one emit block. The backend stores a microflow-backed mode as `CallMicroflow:` and a member fully qualified; DESCRIBE printed the stored forms verbatim. The set/type name confusion is invisible in a single-word case and only shows when the two differ (Studio Pro's convention is singular type, plural set) | `mdl/executor/cmd_odata.go` (`odataModeToMDL`, `bareMemberName`, entity-set exposed name, `KEY` over `IsPartOfKey`) | Storage form ≠ input form: anywhere DESCRIBE prints a value read back from the backend, ask whether the *parser* accepts that spelling — `mx check` and the linter never see DESCRIBE output, so nothing else can catch it. Proved by round trip rather than by eye: describe → check (parses) → drop → exec the output → describe again → **byte-identical**, and the rebuilt model reports 0 errors from mxbuild. Tests `cmd_odata_describe_roundtrip_test.go`. mxcli-formula1 #10.5 | +| A `create database connection … type 'Redshift'` (or `'SQLServer'`) executes, builds **0 errors**, and the connection does not work. The skill's own table listed both, and omitted the one value that matters for an unsupported driver | mxcli passes the type string straight to BSON (`addStr(e,"DatabaseType",…)`) and **mxbuild does not validate it either** — verified on 11.12.1 — so nothing between the author and the runtime says the type is not real. Studio Pro's picker (read from `modeler/ide-client/database-connector-editor/`, identical on 11.10.0/11.12.1/11.13.0) is MSSQL, MySQL, Oracle, PostgreSQL, Snowflake, **BYOD** ("Other") — no Redshift, no SQLServer | `mdl/executor/validate_database_type.go` (`ValidateDatabaseConnectionType`, MDL-DB01), wired in `cmd/mxcli/cmd_check.go`; `.claude/skills/mendix/database-connections.md` | **A warning, not an error**: the value set is version-specific and mxcli cannot prove a string wrong on a Mendix version it has not seen — but silence is worse when the build is green and the connection is dead. **`BYOD` is the discovery worth keeping**: it forces connection-string config and *skips the driver-presence check*, so any JDBC driver Mendix has no entry for (DuckDB, SQLite, ClickHouse) works by dropping the JAR in `userlib/`. **Generalisable**: when a doc lists enum values, the shipped Studio Pro editor bundle is the authority — grep `id:"…",label:"…"` out of `ide-client/`, and diff across cached versions to see whether the set moved. Tests `validate_database_type_test.go`. mxcli-formula1 #6 | diff --git a/.claude/skills/mendix/database-connections.md b/.claude/skills/mendix/database-connections.md index 8aedad889..9f26ff337 100644 --- a/.claude/skills/mendix/database-connections.md +++ b/.claude/skills/mendix/database-connections.md @@ -76,14 +76,31 @@ end; ### Supported Database Types -| Database | TYPE Value | -|----------|------------| -| Oracle | `'Oracle'` | -| PostgreSQL | `'PostgreSQL'` | -| MySQL | `'MySQL'` | -| SQL Server | `'MSSQL'` or `'SQLServer'` | -| Snowflake | `'Snowflake'` | -| Amazon Redshift | `'Redshift'` | +These are the values Studio Pro's own connector editor offers — read out of the +shipped bundle at `modeler/ide-client/database-connector-editor/`, identical on +11.10.0, 11.12.1 and 11.13.0. + +| Database | TYPE Value | Studio Pro label | +|----------|------------|------------------| +| SQL Server | `'MSSQL'` | Microsoft SQL | +| MySQL | `'MySQL'` | MySQL | +| Oracle | `'Oracle'` | Oracle | +| PostgreSQL | `'PostgreSQL'` | PostgreSQL | +| Snowflake | `'Snowflake'` | Snowflake | +| *anything else* | `'BYOD'` | Other | + +**`'BYOD'` — bring your own driver.** Selecting it forces connection-string +configuration and **skips the driver-presence check**; its only validation is +that the connection string is non-empty. That is the hook for any JDBC driver +Mendix ships no picker entry for (DuckDB, SQLite, ClickHouse, …). Drop the +driver JAR in `userlib/` and give the connection its JDBC URL. + +**`'Redshift'` and `'SQLServer'` are not real values.** Both appeared in an +earlier version of this table and neither is in the picker on any version +checked. mxcli writes the type string through unchanged and **mxbuild does not +validate it** — `type 'Redshift'` builds 0 errors and simply does not connect — +so `mxcli check` warns about an unrecognised type (MDL-DB01) rather than letting +a green build hide it. ## Query Definition Syntax diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index a374167db..89705bbb7 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -173,6 +173,11 @@ Examples: // not row-scoped, so the argument is unbound (CE1571) at build time. violations = append(violations, executor.ValidatePageButtonContext(prog)...) + // Flag a database-connection TYPE Studio Pro does not offer. mxcli writes + // the string through and mxbuild does not check it, so a wrong value + // builds green and simply does not connect. + violations = append(violations, executor.ValidateDatabaseConnectionType(prog)...) + // Flag OData property names nothing below will act on. The grammar takes // any `name: value` pair, so a typo used to be discarded in silence and // the model quietly lacked what the author asked for. diff --git a/mdl/executor/validate_database_type.go b/mdl/executor/validate_database_type.go new file mode 100644 index 000000000..4b63a37d9 --- /dev/null +++ b/mdl/executor/validate_database_type.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Check-time (no-project) validation of CREATE DATABASE CONNECTION's TYPE. +// +// mxcli writes the type string straight through to BSON, and mxbuild accepts +// anything: `type 'Redshift'` builds 0 errors on 11.12.1 and is simply not a +// database type Mendix has. The values are the ones Studio Pro's own connector +// editor offers, read out of the shipped bundle at +// modeler/ide-client/database-connector-editor/ (verified identical on 11.10.0, +// 11.12.1 and — per mxcli-formula1 findings #6 — 11.13.0). +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// databaseConnectionTypes is Studio Pro's picker, id -> label. +var databaseConnectionTypes = []struct{ ID, Label string }{ + {"MSSQL", "Microsoft SQL"}, + {"MySQL", "MySQL"}, + {"Oracle", "Oracle"}, + {"PostgreSQL", "PostgreSQL"}, + {"Snowflake", "Snowflake"}, + {"BYOD", "Other — bring your own JDBC driver"}, +} + +// ValidateDatabaseConnectionType warns (MDL-DB01) when a CREATE DATABASE +// CONNECTION names a type Studio Pro does not offer. +// +// A warning rather than an error: the set is version-specific, and mxbuild does +// not reject an unknown value, so mxcli cannot prove one wrong on a Mendix +// version it has not seen. Saying nothing is worse — the build is green and the +// connection simply does not work. +func ValidateDatabaseConnectionType(prog *ast.Program) []linter.Violation { + if prog == nil { + return nil + } + var out []linter.Violation + for _, stmt := range prog.Statements { + s, ok := stmt.(*ast.CreateDatabaseConnectionStmt) + if !ok || s.DatabaseType == "" { + continue + } + if knownDatabaseConnectionType(s.DatabaseType) { + continue + } + out = append(out, linter.Violation{ + RuleID: "MDL-DB01", + Severity: linter.SeverityWarning, + Message: fmt.Sprintf("database connection %s: type %q is not one Studio Pro offers — it is written to the model as-is and mxbuild does not check it, so the build stays green and the connection does not work", + s.Name.String(), s.DatabaseType), + Suggestion: fmt.Sprintf("Use one of: %s. For a JDBC driver Mendix has no entry for, use 'BYOD' — it skips the driver-presence check and takes the connection string as given.", + strings.Join(databaseConnectionTypeIDs(), ", ")), + }) + } + return out +} + +func knownDatabaseConnectionType(name string) bool { + for _, t := range databaseConnectionTypes { + if strings.EqualFold(t.ID, name) { + return true + } + } + return false +} + +func databaseConnectionTypeIDs() []string { + ids := make([]string, 0, len(databaseConnectionTypes)) + for _, t := range databaseConnectionTypes { + ids = append(ids, "'"+t.ID+"'") + } + return ids +} diff --git a/mdl/executor/validate_database_type_test.go b/mdl/executor/validate_database_type_test.go new file mode 100644 index 000000000..7d7510562 --- /dev/null +++ b/mdl/executor/validate_database_type_test.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// mxcli-formula1 findings #6: the skill documented `Redshift` (not in Studio +// Pro's picker on any version checked) and omitted `BYOD` (the only way to use +// a JDBC driver Mendix ships no entry for). mxcli writes the string straight +// through and mxbuild does not check it — `type 'Redshift'` builds 0 errors on +// 11.12.1 — so a wrong value hides behind a green build. +func TestValidateDatabaseConnectionType(t *testing.T) { + script := func(dbType string) string { + return ` +create module D; +create constant D.Cs type string default 'jdbc:x'; +create constant D.U type string default 'u'; +create constant D.P type string default 'p'; +create database connection D.Conn +type '` + dbType + `' +connection string @D.Cs +username @D.U +password @D.P +begin +end; +` + } + + tests := []struct { + dbType string + wantWarn bool + }{ + {"PostgreSQL", false}, + {"MSSQL", false}, + {"Oracle", false}, + {"MySQL", false}, + {"Snowflake", false}, + {"BYOD", false}, + // Matching is case-insensitive: the picker's id is the canonical + // spelling, but a different casing is not a different type. + {"postgresql", false}, + // Both of these were in the skill's table and neither is real. + {"Redshift", true}, + {"SQLServer", true}, + {"Postgres", true}, + } + + for _, tt := range tests { + t.Run(tt.dbType, func(t *testing.T) { + prog, errs := visitor.Build(script(tt.dbType)) + if len(errs) > 0 { + t.Fatalf("parsing the script: %v", errs) + } + got := ValidateDatabaseConnectionType(prog) + if tt.wantWarn { + if len(got) != 1 { + t.Fatalf("expected 1 warning for %q, got %d: %v", tt.dbType, len(got), got) + } + if got[0].RuleID != "MDL-DB01" { + t.Errorf("rule = %q, want MDL-DB01", got[0].RuleID) + } + // A warning, not an error: the set is version-specific and + // mxcli cannot prove a value wrong on a version it has not seen. + if got[0].Severity != linter.SeverityWarning { + t.Errorf("severity = %v, want warning", got[0].Severity) + } + // BYOD is the way out for a driver Mendix has no entry for, so + // the suggestion has to name it. + if !strings.Contains(got[0].Suggestion, "BYOD") { + t.Errorf("suggestion should point at BYOD, got: %s", got[0].Suggestion) + } + } else if len(got) != 0 { + t.Errorf("expected no warning for %q, got: %v", tt.dbType, got) + } + }) + } +} From 83e85c2c6bc4ae952fb12674198731177f6bd8bd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:09:28 +0000 Subject: [PATCH 07/10] fix(init): stop inventing a project when the directory has none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run from a solution root — one folder per app, no .mpr at the root — `mxcli init` reported success and wrote tooling pointing at `project.mpr`, a file that does not exist. The hardcoded default made a missing project look like a found one. It now looks one level down and lets the candidate count decide: none, warn that the generated paths are placeholders; exactly one, say which project it is and initialise that directory; more than one, refuse, list them, and print the command naming a specific app. A solution repo should not get one app's tooling by coin flip. One level only. A Mendix app keeps its .mpr at its own root, so walking deeper would start finding deployment copies and backups. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + cmd/mxcli/init.go | 51 ++++++++++++++++++++++- cmd/mxcli/init_discover_test.go | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 cmd/mxcli/init_discover_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index a3ec58155..96b4163bc 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -396,3 +396,4 @@ extracting `OffsetExpression`/`LimitExpression`. | A read-microflow-backed OData resource must declare a `System.ODataResponse` parameter and compute a count, even when the count is expensive (a full CSV scan) and nobody asked for it — with no MDL to say otherwise. Same for `$skip`/`$top` support | `Countable`, `SkipSupported` and `TopSupported` were written as literal `true` in the BSON writer's `ODataPublish$QueryOptions`; nothing above the writer could express them | `mdl/ast/ast_odata.go` + `mdl/visitor/visitor_odata.go` (`*bool` on `PublishedEntityDef`, `odataBoolPtr`), `model/types.go`, `mdl/executor/cmd_odata.go` (`astEntityDefToModel`), `mdl/backend/modelsdk/odata_write.go` (`boolOrDefault`), `odata_read_detail.go` (`falseOnly`) | Tri-state (`*bool`) is load-bearing: these default to **true**, so "unset" and "false" cannot share a representation or every existing script would silently turn them off. The reader maps a stored `true` back to nil (`falseOnly`) so DESCRIBE prints only what the author wrote instead of three defaults on every resource. Verified end to end on 11.12.1: `Countable: No` + a read microflow with **no** `$Response` parameter builds 0 errors, which is exactly the combination that was impossible before. Tests `cmd_odata_query_options_test.go`, `odata_write_test.go`. mxcli-formula1 #10.3 | | `describe odata service Module.Api` emits MDL that will not parse — `ReadMode: CallMicroflow:Module.Read` matches no value, `expose (Module.Entity.Attr ...)` where the clause takes a bare member name — and quietly renames the entity set, printing the entity TYPE's exposed name in the `as '…'` position where the entity SET's belongs | Three independent slips in one emit block. The backend stores a microflow-backed mode as `CallMicroflow:` and a member fully qualified; DESCRIBE printed the stored forms verbatim. The set/type name confusion is invisible in a single-word case and only shows when the two differ (Studio Pro's convention is singular type, plural set) | `mdl/executor/cmd_odata.go` (`odataModeToMDL`, `bareMemberName`, entity-set exposed name, `KEY` over `IsPartOfKey`) | Storage form ≠ input form: anywhere DESCRIBE prints a value read back from the backend, ask whether the *parser* accepts that spelling — `mx check` and the linter never see DESCRIBE output, so nothing else can catch it. Proved by round trip rather than by eye: describe → check (parses) → drop → exec the output → describe again → **byte-identical**, and the rebuilt model reports 0 errors from mxbuild. Tests `cmd_odata_describe_roundtrip_test.go`. mxcli-formula1 #10.5 | | A `create database connection … type 'Redshift'` (or `'SQLServer'`) executes, builds **0 errors**, and the connection does not work. The skill's own table listed both, and omitted the one value that matters for an unsupported driver | mxcli passes the type string straight to BSON (`addStr(e,"DatabaseType",…)`) and **mxbuild does not validate it either** — verified on 11.12.1 — so nothing between the author and the runtime says the type is not real. Studio Pro's picker (read from `modeler/ide-client/database-connector-editor/`, identical on 11.10.0/11.12.1/11.13.0) is MSSQL, MySQL, Oracle, PostgreSQL, Snowflake, **BYOD** ("Other") — no Redshift, no SQLServer | `mdl/executor/validate_database_type.go` (`ValidateDatabaseConnectionType`, MDL-DB01), wired in `cmd/mxcli/cmd_check.go`; `.claude/skills/mendix/database-connections.md` | **A warning, not an error**: the value set is version-specific and mxcli cannot prove a string wrong on a Mendix version it has not seen — but silence is worse when the build is green and the connection is dead. **`BYOD` is the discovery worth keeping**: it forces connection-string config and *skips the driver-presence check*, so any JDBC driver Mendix has no entry for (DuckDB, SQLite, ClickHouse) works by dropping the JAR in `userlib/`. **Generalisable**: when a doc lists enum values, the shipped Studio Pro editor bundle is the authority — grep `id:"…",label:"…"` out of `ide-client/`, and diff across cached versions to see whether the set moved. Tests `validate_database_type_test.go`. mxcli-formula1 #6 | +| `mxcli init` run from a solution root (several app folders, no `.mpr` at the root) reports success and writes tooling that points at `project.mpr` — a file that does not exist. Nobody is told which project it picked, because it did not pick one | `findMprFile` looks only in the target directory; an empty result fell through to a hardcoded `"project.mpr"` default and initialisation continued as if that were a real project | `cmd/mxcli/init.go` (`findMprFilesInSubdirs` + the three-way branch on the candidate count) | Look one level down, then branch on the **count**: 0 → warn that generated paths will be placeholders; 1 → announce the project and initialise **that** directory; 2+ → refuse, list them, and print the exact command naming one. One level only — a Mendix app keeps its `.mpr` at its own root, and walking deeper starts finding deployment copies and backups. Candidates are sorted so the refusal and its suggested command are stable rather than directory-order dependent. **Generalisable**: a "sensible default" that names a file which does not exist is not a default, it is a silent wrong answer — count the candidates and let the count choose the behaviour. Tests `cmd/mxcli/init_discover_test.go`. mxcli-formula1 #3 | diff --git a/cmd/mxcli/init.go b/cmd/mxcli/init.go index 13e4e84d8..ca43f9a5c 100644 --- a/cmd/mxcli/init.go +++ b/cmd/mxcli/init.go @@ -10,6 +10,7 @@ import ( "path/filepath" "runtime" "slices" + "sort" "strings" "github.com/mendixlabs/mxcli/mdl/linter" @@ -134,10 +135,33 @@ Container Runtime: os.Exit(1) } - // Find .mpr file + // Find .mpr file. With none here, look one level down: a solution repo + // keeps each app in its own folder, and running `mxcli init` from the + // root used to write everything at the root against an invented + // `project.mpr` that does not exist — silently wrong, and in a two-app + // repo the odds of it being what you meant are zero. + // (mxcli-formula1 findings #3.) mprFile := findMprFile(absDir) if mprFile == "" { - mprFile = "project.mpr" // Default if not found + candidates := findMprFilesInSubdirs(absDir) + switch len(candidates) { + case 0: + fmt.Fprintf(os.Stderr, "Warning: no .mpr file found in %s.\n", absDir) + fmt.Fprintln(os.Stderr, " Generated files will refer to 'project.mpr'; run this from the app folder to get real paths.") + mprFile = "project.mpr" + case 1: + absDir = filepath.Dir(candidates[0]) + mprFile = filepath.Base(candidates[0]) + fmt.Printf("No .mpr here; initializing the project found below: %s\n", candidates[0]) + default: + fmt.Fprintf(os.Stderr, "Error: %d Mendix projects found below %s:\n", len(candidates), absDir) + for _, c := range candidates { + fmt.Fprintf(os.Stderr, " %s\n", c) + } + fmt.Fprintln(os.Stderr, "\nName the one you mean, so a solution repo does not get one app's tooling by coin flip:") + fmt.Fprintf(os.Stderr, " mxcli init %s\n", filepath.Dir(candidates[0])) + os.Exit(1) + } } projectName := filepath.Base(absDir) @@ -673,3 +697,26 @@ func init() { initCmd.Flags().BoolVar(&initListTools, "list-tools", false, "List supported AI tools and exit") initCmd.Flags().StringVar(&initContainerRuntime, "container-runtime", "docker", "Container runtime for devcontainer (docker or podman)") } + +// findMprFilesInSubdirs returns the .mpr files one level below dir, sorted, so +// the choice is deterministic and the error message lists them in a stable +// order. One level only: a Mendix app keeps its .mpr at its root, and walking +// deeper would find deployment copies and backups. +func findMprFilesInSubdirs(dir string) []string { + entries, err := os.ReadDir(dir) + if err != nil { + return nil + } + var out []string + for _, e := range entries { + if !e.IsDir() || strings.HasPrefix(e.Name(), ".") { + continue + } + sub := filepath.Join(dir, e.Name()) + if mpr := findMprFile(sub); mpr != "" { + out = append(out, filepath.Join(sub, mpr)) + } + } + sort.Strings(out) + return out +} diff --git a/cmd/mxcli/init_discover_test.go b/cmd/mxcli/init_discover_test.go new file mode 100644 index 000000000..f21bd3a33 --- /dev/null +++ b/cmd/mxcli/init_discover_test.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// mxcli-formula1 findings #3: with no .mpr at the target, `mxcli init` used to +// carry on against an invented `project.mpr`, writing tooling that points at a +// file which does not exist. In a solution repo — one app folder per app — that +// is silently the wrong answer, and with two apps there is no right guess. +func TestFindMprFilesInSubdirs(t *testing.T) { + t.Run("finds one project per subdirectory, sorted", func(t *testing.T) { + root := t.TempDir() + for _, name := range []string{"Zeta", "Alpha"} { + dir := filepath.Join(root, name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name+".mpr"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + got := findMprFilesInSubdirs(root) + if len(got) != 2 { + t.Fatalf("got %d candidates, want 2: %v", len(got), got) + } + // Sorted, so the refusal message and the suggested command are stable + // rather than dependent on directory order. + if filepath.Base(got[0]) != "Alpha.mpr" || filepath.Base(got[1]) != "Zeta.mpr" { + t.Errorf("candidates are not sorted: %v", got) + } + }) + + t.Run("ignores dot directories", func(t *testing.T) { + root := t.TempDir() + hidden := filepath.Join(root, ".mendix-cache") + if err := os.MkdirAll(hidden, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(hidden, "stale.mpr"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if got := findMprFilesInSubdirs(root); len(got) != 0 { + t.Errorf("a dot directory is not a candidate project, got: %v", got) + } + }) + + t.Run("one level only", func(t *testing.T) { + root := t.TempDir() + deep := filepath.Join(root, "apps", "Nested") + if err := os.MkdirAll(deep, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(deep, "Nested.mpr"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + // A Mendix app keeps its .mpr at its own root; walking deeper would + // start finding deployment copies and backups. + if got := findMprFilesInSubdirs(root); len(got) != 0 { + t.Errorf("expected no candidates two levels down, got: %v", got) + } + }) + + t.Run("empty directory", func(t *testing.T) { + if got := findMprFilesInSubdirs(t.TempDir()); len(got) != 0 { + t.Errorf("got %v, want none", got) + } + }) +} From f0d9e38495056787a73a70856de8427629b7b369 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:19:20 +0000 Subject: [PATCH 08/10] feat(settings): read one configuration, and show its root URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `alter settings configuration 'Default' …` is the write form; the read form was a parse error. `describe settings configuration ''` now parses and prints that one configuration as re-executable MDL, naming the configurations that do exist when the name is wrong. Where MDL has `alter X `, reaching for `describe X ` and getting a parse error teaches the wrong lesson. `show settings configurations` also gains ApplicationRootUrl. It decides the host the app answers on, so "did my root URL land?" is the obvious question to ask that command, and the summary could not answer it — `describe settings | grep` was the only way. An empty DatabaseUrl is skipped too, rather than rendering as a bare comma that reads like a bug in the reader. Both dumps go through one emit helper, so the whole-settings output and the single-configuration output cannot drift apart. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + cmd/mxcli/syntax/features_misc.go | 6 +- mdl/executor/cmd_notconnected_mock_test.go | 2 +- mdl/executor/cmd_odata_mock_test.go | 4 +- mdl/executor/cmd_settings.go | 100 +++++++++++----- .../cmd_settings_configuration_test.go | 111 ++++++++++++++++++ mdl/executor/cmd_settings_mock_test.go | 4 +- mdl/executor/cmd_settings_private_test.go | 2 +- mdl/executor/executor_query.go | 2 +- mdl/grammar/domains/MDLCatalog.g4 | 2 +- mdl/visitor/visitor_query.go | 15 ++- 11 files changed, 205 insertions(+), 44 deletions(-) create mode 100644 mdl/executor/cmd_settings_configuration_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 96b4163bc..3c1cf47c0 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -397,3 +397,4 @@ extracting `OffsetExpression`/`LimitExpression`. | `describe odata service Module.Api` emits MDL that will not parse — `ReadMode: CallMicroflow:Module.Read` matches no value, `expose (Module.Entity.Attr ...)` where the clause takes a bare member name — and quietly renames the entity set, printing the entity TYPE's exposed name in the `as '…'` position where the entity SET's belongs | Three independent slips in one emit block. The backend stores a microflow-backed mode as `CallMicroflow:` and a member fully qualified; DESCRIBE printed the stored forms verbatim. The set/type name confusion is invisible in a single-word case and only shows when the two differ (Studio Pro's convention is singular type, plural set) | `mdl/executor/cmd_odata.go` (`odataModeToMDL`, `bareMemberName`, entity-set exposed name, `KEY` over `IsPartOfKey`) | Storage form ≠ input form: anywhere DESCRIBE prints a value read back from the backend, ask whether the *parser* accepts that spelling — `mx check` and the linter never see DESCRIBE output, so nothing else can catch it. Proved by round trip rather than by eye: describe → check (parses) → drop → exec the output → describe again → **byte-identical**, and the rebuilt model reports 0 errors from mxbuild. Tests `cmd_odata_describe_roundtrip_test.go`. mxcli-formula1 #10.5 | | A `create database connection … type 'Redshift'` (or `'SQLServer'`) executes, builds **0 errors**, and the connection does not work. The skill's own table listed both, and omitted the one value that matters for an unsupported driver | mxcli passes the type string straight to BSON (`addStr(e,"DatabaseType",…)`) and **mxbuild does not validate it either** — verified on 11.12.1 — so nothing between the author and the runtime says the type is not real. Studio Pro's picker (read from `modeler/ide-client/database-connector-editor/`, identical on 11.10.0/11.12.1/11.13.0) is MSSQL, MySQL, Oracle, PostgreSQL, Snowflake, **BYOD** ("Other") — no Redshift, no SQLServer | `mdl/executor/validate_database_type.go` (`ValidateDatabaseConnectionType`, MDL-DB01), wired in `cmd/mxcli/cmd_check.go`; `.claude/skills/mendix/database-connections.md` | **A warning, not an error**: the value set is version-specific and mxcli cannot prove a string wrong on a Mendix version it has not seen — but silence is worse when the build is green and the connection is dead. **`BYOD` is the discovery worth keeping**: it forces connection-string config and *skips the driver-presence check*, so any JDBC driver Mendix has no entry for (DuckDB, SQLite, ClickHouse) works by dropping the JAR in `userlib/`. **Generalisable**: when a doc lists enum values, the shipped Studio Pro editor bundle is the authority — grep `id:"…",label:"…"` out of `ide-client/`, and diff across cached versions to see whether the set moved. Tests `validate_database_type_test.go`. mxcli-formula1 #6 | | `mxcli init` run from a solution root (several app folders, no `.mpr` at the root) reports success and writes tooling that points at `project.mpr` — a file that does not exist. Nobody is told which project it picked, because it did not pick one | `findMprFile` looks only in the target directory; an empty result fell through to a hardcoded `"project.mpr"` default and initialisation continued as if that were a real project | `cmd/mxcli/init.go` (`findMprFilesInSubdirs` + the three-way branch on the candidate count) | Look one level down, then branch on the **count**: 0 → warn that generated paths will be placeholders; 1 → announce the project and initialise **that** directory; 2+ → refuse, list them, and print the exact command naming one. One level only — a Mendix app keeps its `.mpr` at its own root, and walking deeper starts finding deployment copies and backups. Candidates are sorted so the refusal and its suggested command are stable rather than directory-order dependent. **Generalisable**: a "sensible default" that names a file which does not exist is not a default, it is a silent wrong answer — count the candidates and let the count choose the behaviour. Tests `cmd/mxcli/init_discover_test.go`. mxcli-formula1 #3 | +| `alter settings configuration 'Default' …` is the write form, but `describe settings configuration 'Default'` is a **parse error** — and `show settings configurations` summarises the configuration without `ApplicationRootUrl`, so the obvious command for "did my root URL land?" cannot answer it (and renders an empty DatabaseUrl as a bare `, ,`) | The grammar's `DESCRIBE SETTINGS` alternative took no object, so the read form of a write statement simply did not exist; the summary builder listed database/port fields and never gained the root URL when that property was added | `mdl/grammar/domains/MDLCatalog.g4` (`DESCRIBE SETTINGS (CONFIGURATION STRING_LITERAL)?`), `mdl/visitor/visitor_query.go` (reuses `DescribeStmt.Qualifier`), `mdl/executor/cmd_settings.go` (`writeSettingsConfiguration`, `describeSettingsConfiguration`, summary) | **Read forms should mirror write forms** — where MDL has `alter X `, `describe X ` should parse, and reaching for it and getting a parse error teaches the wrong lesson. Factor the emit into one helper so the whole-settings dump and the single-configuration dump cannot drift. An unknown name lists the ones that exist, or the user is guessing. **Watch for**: changing `describeSettings`'s signature broke three existing callers, and the same commit's `IsPartOfKey`→`KEY` change broke an OData round-trip test that only the FULL suite caught — run `go test ./mdl/...`, not just the new test. Tests `cmd_settings_configuration_test.go`. mxcli-formula1 #8 | diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 1a3f4f108..85560be1e 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -125,7 +125,7 @@ DISCONNECT;`, "settings", "project settings", "configuration", "startup", "shutdown", "hash algorithm", "java version", }, - Syntax: "SHOW SETTINGS;\nDESCRIBE SETTINGS;\nALTER SETTINGS MODEL = ;\nALTER SETTINGS CONFIGURATION '' = ;", + Syntax: "SHOW SETTINGS;\nDESCRIBE SETTINGS;\nDESCRIBE SETTINGS CONFIGURATION ''; -- just one configuration\nALTER SETTINGS MODEL = ;\nALTER SETTINGS CONFIGURATION '' = ;", Example: "SHOW SETTINGS;\nALTER SETTINGS MODEL AfterStartupMicroflow = 'Module.MF_Startup';", SeeAlso: []string{"settings.show", "settings.alter"}, }) @@ -136,8 +136,8 @@ DISCONNECT;`, Keywords: []string{ "show settings", "describe settings", "list settings", }, - Syntax: "SHOW SETTINGS;\nDESCRIBE SETTINGS;", - Example: "SHOW SETTINGS;\nDESCRIBE SETTINGS;", + Syntax: "SHOW SETTINGS;\nDESCRIBE SETTINGS;\nDESCRIBE SETTINGS CONFIGURATION '';", + Example: "SHOW SETTINGS;\nDESCRIBE SETTINGS;\nDESCRIBE SETTINGS CONFIGURATION 'Default';", }) Register(SyntaxFeature{ diff --git a/mdl/executor/cmd_notconnected_mock_test.go b/mdl/executor/cmd_notconnected_mock_test.go index b2bae29da..662d93b1c 100644 --- a/mdl/executor/cmd_notconnected_mock_test.go +++ b/mdl/executor/cmd_notconnected_mock_test.go @@ -93,7 +93,7 @@ func TestDescribeMermaid_Mock_NotConnected(t *testing.T) { func TestDescribeSettings_Mock_NotConnected(t *testing.T) { ctx, _ := newMockCtx(t, withBackend(disconnectedBackend())) - assertError(t, describeSettings(ctx)) + assertError(t, describeSettings(ctx, "")) } func TestDescribeBusinessEventService_Mock_NotConnected(t *testing.T) { diff --git a/mdl/executor/cmd_odata_mock_test.go b/mdl/executor/cmd_odata_mock_test.go index ebdad1cac..dd2214b15 100644 --- a/mdl/executor/cmd_odata_mock_test.go +++ b/mdl/executor/cmd_odata_mock_test.go @@ -462,7 +462,9 @@ func TestDescribeODataService_ExposeRoundtrip(t *testing.T) { assertNoError(t, describeODataService(ctx, ast.QualifiedName{Module: "MyModule", Name: "CatalogService"})) out := buf.String() - assertContainsStr(t, out, "IsPartOfKey") + // KEY, not IsPartOfKey: both parse, but DESCRIBE emits the spelling the + // syntax help documents (mxcli-formula1 #10.5). + assertContainsStr(t, out, "(KEY)") _, errs := visitor.Build(out) if len(errs) > 0 { diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index 7dc728d71..f7a990a0a 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -44,9 +44,19 @@ func listSettings(ctx *ExecContext) error { for _, cfg := range ps.Configuration.Configurations { values := []string{} values = append(values, cfg.DatabaseType) - values = append(values, cfg.DatabaseUrl) + // An empty DatabaseUrl used to render as a bare ", ," — a gap where + // a value should be, which reads as a bug in the reader. + if cfg.DatabaseUrl != "" { + values = append(values, cfg.DatabaseUrl) + } values = append(values, "db="+cfg.DatabaseName) values = append(values, fmt.Sprintf("http=%d", cfg.HttpPortNumber)) + // The root URL decides the host the app answers on, so "did my root + // URL land?" is the obvious question to ask this command — and the + // summary used to be unable to answer it (mxcli-formula1 #8). + if cfg.ApplicationRootUrl != "" { + values = append(values, "url="+cfg.ApplicationRootUrl) + } if len(cfg.ConstantValues) > 0 { values = append(values, fmt.Sprintf("%d constants", len(cfg.ConstantValues))) } @@ -85,7 +95,11 @@ func listSettings(ctx *ExecContext) error { } // describeSettings outputs the full MDL description of all settings. -func describeSettings(ctx *ExecContext) error { +// describeSettings prints the project settings as re-executable `alter settings` +// statements. With configName set (DESCRIBE SETTINGS CONFIGURATION 'X') it +// prints only that configuration — the read form of `alter settings +// configuration 'X'`, which used to be a parse error. +func describeSettings(ctx *ExecContext, configName string) error { if !ctx.Connected() { return mdlerrors.NewNotConnected() } @@ -95,6 +109,10 @@ func describeSettings(ctx *ExecContext) error { return mdlerrors.NewBackend("read project settings", err) } + if configName != "" { + return describeSettingsConfiguration(ctx, ps, configName) + } + // Model settings if ps.Model != nil { ms := ps.Model @@ -122,34 +140,7 @@ func describeSettings(ctx *ExecContext) error { // Configuration settings if ps.Configuration != nil { for _, cfg := range ps.Configuration.Configurations { - var parts []string - parts = append(parts, fmt.Sprintf(" DatabaseType = '%s'", cfg.DatabaseType)) - parts = append(parts, fmt.Sprintf(" DatabaseUrl = '%s'", cfg.DatabaseUrl)) - parts = append(parts, fmt.Sprintf(" DatabaseName = '%s'", cfg.DatabaseName)) - parts = append(parts, fmt.Sprintf(" DatabaseUserName = '%s'", cfg.DatabaseUserName)) - parts = append(parts, fmt.Sprintf(" DatabasePassword = '%s'", cfg.DatabasePassword)) - parts = append(parts, fmt.Sprintf(" HttpPortNumber = %d", cfg.HttpPortNumber)) - parts = append(parts, fmt.Sprintf(" ServerPortNumber = %d", cfg.ServerPortNumber)) - if cfg.ApplicationRootUrl != "" { - parts = append(parts, fmt.Sprintf(" ApplicationRootUrl = '%s'", cfg.ApplicationRootUrl)) - } - fmt.Fprintf(ctx.Output, "alter settings configuration '%s'\n%s;\n\n", cfg.Name, strings.Join(parts, ",\n")) - - // Output constant overrides. A private override has no value in the - // model — emitting `value ''` would round-trip into a *shared* empty - // override, moving a value that is deliberately kept off the shared model - // into it. MDL does not author the shared/private choice, so describe - // reports it as a comment instead of a re-executable statement. - for _, cv := range cfg.ConstantValues { - if cv.IsPrivate { - fmt.Fprintf(ctx.Output, "-- constant '%s' has a private value in configuration '%s'\n"+ - "-- (stored on the developer's workstation; not part of the shared model)\n\n", - cv.ConstantId, cfg.Name) - continue - } - fmt.Fprintf(ctx.Output, "alter settings constant '%s' value '%s'\n in configuration '%s';\n\n", - cv.ConstantId, cv.Value, cfg.Name) - } + writeSettingsConfiguration(ctx, cfg) } } @@ -645,3 +636,52 @@ func settingsValueToString(val any) string { return fmt.Sprintf("%v", v) } } + +// writeSettingsConfiguration emits one configuration as re-executable MDL. +func writeSettingsConfiguration(ctx *ExecContext, cfg *model.ServerConfiguration) { + var parts []string + parts = append(parts, fmt.Sprintf(" DatabaseType = '%s'", cfg.DatabaseType)) + parts = append(parts, fmt.Sprintf(" DatabaseUrl = '%s'", cfg.DatabaseUrl)) + parts = append(parts, fmt.Sprintf(" DatabaseName = '%s'", cfg.DatabaseName)) + parts = append(parts, fmt.Sprintf(" DatabaseUserName = '%s'", cfg.DatabaseUserName)) + parts = append(parts, fmt.Sprintf(" DatabasePassword = '%s'", cfg.DatabasePassword)) + parts = append(parts, fmt.Sprintf(" HttpPortNumber = %d", cfg.HttpPortNumber)) + parts = append(parts, fmt.Sprintf(" ServerPortNumber = %d", cfg.ServerPortNumber)) + if cfg.ApplicationRootUrl != "" { + parts = append(parts, fmt.Sprintf(" ApplicationRootUrl = '%s'", cfg.ApplicationRootUrl)) + } + fmt.Fprintf(ctx.Output, "alter settings configuration '%s'\n%s;\n\n", cfg.Name, strings.Join(parts, ",\n")) + + // Output constant overrides. A private override has no value in the + // model — emitting `value ''` would round-trip into a *shared* empty + // override, moving a value that is deliberately kept off the shared model + // into it. MDL does not author the shared/private choice, so describe + // reports it as a comment instead of a re-executable statement. + for _, cv := range cfg.ConstantValues { + if cv.IsPrivate { + fmt.Fprintf(ctx.Output, "-- constant '%s' has a private value in configuration '%s'\n"+ + "-- (stored on the developer's workstation; not part of the shared model)\n\n", + cv.ConstantId, cfg.Name) + continue + } + fmt.Fprintf(ctx.Output, "alter settings constant '%s' value '%s'\n in configuration '%s';\n\n", + cv.ConstantId, cv.Value, cfg.Name) + } +} + +// describeSettingsConfiguration prints a single named configuration, or names +// the ones that exist when the requested name is not among them. +func describeSettingsConfiguration(ctx *ExecContext, ps *model.ProjectSettings, name string) error { + var available []string + if ps.Configuration != nil { + for _, cfg := range ps.Configuration.Configurations { + if strings.EqualFold(cfg.Name, name) { + writeSettingsConfiguration(ctx, cfg) + return nil + } + available = append(available, "'"+cfg.Name+"'") + } + } + return mdlerrors.NewNotFoundMsg("settings configuration", name, + fmt.Sprintf("settings configuration not found: '%s' (available: %s)", name, strings.Join(available, ", "))) +} diff --git a/mdl/executor/cmd_settings_configuration_test.go b/mdl/executor/cmd_settings_configuration_test.go new file mode 100644 index 000000000..bdf5bb77b --- /dev/null +++ b/mdl/executor/cmd_settings_configuration_test.go @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" +) + +// mxcli-formula1 findings #8: `alter settings configuration 'X'` is the write +// form, but the read form was a parse error, and the `show` summary omitted +// ApplicationRootUrl — so the obvious command for "did my root URL land?" could +// not answer it. +func settingsCtxWithConfigs(t *testing.T) *ExecContext { + t.Helper() + ps := &model.ProjectSettings{ + Configuration: &model.ConfigurationSettings{ + Configurations: []*model.ServerConfiguration{ + { + Name: "Default", DatabaseType: "Hsqldb", DatabaseName: "default", + HttpPortNumber: 8080, ApplicationRootUrl: "http://backend.local:8080/", + }, + {Name: "Test", DatabaseType: "PostgreSQL", DatabaseName: "test", HttpPortNumber: 8180}, + }, + }, + } + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return ps, nil }, + } + ctx, _ := newMockCtx(t, withBackend(mb)) + return ctx +} + +func TestDescribeSettingsConfiguration_ByName(t *testing.T) { + ctx := settingsCtxWithConfigs(t) + buf := ctx.Output.(*bytes.Buffer) + if err := describeSettings(ctx, "Default"); err != nil { + t.Fatal(err) + } + out := buf.String() + + if !strings.Contains(out, "alter settings configuration 'Default'") { + t.Errorf("expected the named configuration, got:\n%s", out) + } + if !strings.Contains(out, "ApplicationRootUrl = 'http://backend.local:8080/'") { + t.Errorf("expected the root URL, got:\n%s", out) + } + // Naming one configuration means one configuration, not all of them. + if strings.Contains(out, "'Test'") { + t.Errorf("expected only the named configuration, got:\n%s", out) + } +} + +// Case-insensitive, like the rest of MDL's name matching. +func TestDescribeSettingsConfiguration_CaseInsensitive(t *testing.T) { + ctx := settingsCtxWithConfigs(t) + if err := describeSettings(ctx, "default"); err != nil { + t.Fatalf("expected a case-insensitive match: %v", err) + } +} + +// A wrong name has to say which ones exist, or the user is guessing. +func TestDescribeSettingsConfiguration_UnknownNameListsAvailable(t *testing.T) { + ctx := settingsCtxWithConfigs(t) + err := describeSettings(ctx, "Nope") + if err == nil { + t.Fatal("expected an error for an unknown configuration") + } + for _, want := range []string{"Nope", "Default", "Test"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should mention %q, got: %v", want, err) + } + } +} + +// No name still prints everything, as before. +func TestDescribeSettings_AllConfigurations(t *testing.T) { + ctx := settingsCtxWithConfigs(t) + buf := ctx.Output.(*bytes.Buffer) + if err := describeSettings(ctx, ""); err != nil { + t.Fatal(err) + } + out := buf.String() + for _, want := range []string{"'Default'", "'Test'"} { + if !strings.Contains(out, want) { + t.Errorf("expected every configuration, %q missing from:\n%s", want, out) + } + } +} + +func TestShowSettings_SummaryCarriesRootURL(t *testing.T) { + ctx := settingsCtxWithConfigs(t) + buf := ctx.Output.(*bytes.Buffer) + if err := listSettings(ctx); err != nil { + t.Fatal(err) + } + out := buf.String() + if !strings.Contains(out, "url=http://backend.local:8080/") { + t.Errorf("summary should carry the root URL, got:\n%s", out) + } + // An empty DatabaseUrl leaves a gap where a value should be; it is omitted + // rather than rendered as a bare comma. + if strings.Contains(out, "Hsqldb, , ") { + t.Errorf("summary should skip an empty DatabaseUrl, got:\n%s", out) + } +} diff --git a/mdl/executor/cmd_settings_mock_test.go b/mdl/executor/cmd_settings_mock_test.go index d33aecaaa..be138f14f 100644 --- a/mdl/executor/cmd_settings_mock_test.go +++ b/mdl/executor/cmd_settings_mock_test.go @@ -44,7 +44,7 @@ func TestDescribeSettings_Mock(t *testing.T) { }, } ctx, buf := newMockCtx(t, withBackend(mb)) - assertNoError(t, describeSettings(ctx)) + assertNoError(t, describeSettings(ctx, "")) assertContainsStr(t, buf.String(), "alter settings") } @@ -61,7 +61,7 @@ func TestDescribeSettings_NotConnected(t *testing.T) { IsConnectedFunc: func() bool { return false }, } ctx, _ := newMockCtx(t, withBackend(mb)) - assertError(t, describeSettings(ctx)) + assertError(t, describeSettings(ctx, "")) } func TestShowSettings_BackendError(t *testing.T) { diff --git a/mdl/executor/cmd_settings_private_test.go b/mdl/executor/cmd_settings_private_test.go index facacf18a..3a1677775 100644 --- a/mdl/executor/cmd_settings_private_test.go +++ b/mdl/executor/cmd_settings_private_test.go @@ -110,7 +110,7 @@ func TestDescribeSettings_PrivateOverrideIsNotReExecutable(t *testing.T) { wrote := false ctx, out := newMockCtx(t, withBackend(privateConstantBackend(&wrote))) - if err := describeSettings(ctx); err != nil { + if err := describeSettings(ctx, ""); err != nil { t.Fatalf("describeSettings: %v", err) } got := out.String() diff --git a/mdl/executor/executor_query.go b/mdl/executor/executor_query.go index 86e6a45db..b7bcaab41 100644 --- a/mdl/executor/executor_query.go +++ b/mdl/executor/executor_query.go @@ -226,7 +226,7 @@ func execDescribe(ctx *ExecContext, s *ast.DescribeStmt) error { case ast.DescribeDatabaseConnection: return describeDatabaseConnection(ctx, s.Name) case ast.DescribeSettings: - return describeSettings(ctx) + return describeSettings(ctx, s.Qualifier) case ast.DescribeFragment: return describeFragment(ctx, s.Name) case ast.DescribeImageCollection: diff --git a/mdl/grammar/domains/MDLCatalog.g4 b/mdl/grammar/domains/MDLCatalog.g4 index d367099a1..9cd613bbd 100644 --- a/mdl/grammar/domains/MDLCatalog.g4 +++ b/mdl/grammar/domains/MDLCatalog.g4 @@ -158,7 +158,7 @@ describeStatement | DESCRIBE CATALOG DOT (catalogTableName) // DESCRIBE CATALOG.ENTITIES | DESCRIBE BUSINESS EVENT SERVICE qualifiedName // DESCRIBE BUSINESS EVENT SERVICE Module.Name | DESCRIBE DATABASE CONNECTION qualifiedName // DESCRIBE DATABASE CONNECTION Module.Name - | DESCRIBE SETTINGS // DESCRIBE SETTINGS + | DESCRIBE SETTINGS (CONFIGURATION STRING_LITERAL)? // DESCRIBE SETTINGS [CONFIGURATION 'Default'] | DESCRIBE FRAGMENT FROM PAGE qualifiedName WIDGET identifierOrKeyword // DESCRIBE FRAGMENT FROM PAGE Module.Page WIDGET name | DESCRIBE FRAGMENT FROM SNIPPET qualifiedName WIDGET identifierOrKeyword // DESCRIBE FRAGMENT FROM SNIPPET Module.Snippet WIDGET name | DESCRIBE IMAGE COLLECTION qualifiedName // DESCRIBE IMAGE COLLECTION Module.Name diff --git a/mdl/visitor/visitor_query.go b/mdl/visitor/visitor_query.go index 64d8727c5..8ae708a13 100644 --- a/mdl/visitor/visitor_query.go +++ b/mdl/visitor/visitor_query.go @@ -800,11 +800,18 @@ func (b *Builder) ExitDescribeStatement(ctx *parser.DescribeStatementContext) { return } - // Handle DESCRIBE SETTINGS + // Handle DESCRIBE SETTINGS [CONFIGURATION 'Name'] if ctx.SETTINGS() != nil { - b.statements = append(b.statements, &ast.DescribeStmt{ - ObjectType: ast.DescribeSettings, - }) + stmt := &ast.DescribeStmt{ObjectType: ast.DescribeSettings} + // `alter settings configuration 'X'` is the write form, so the read + // form has to accept the same shape — reaching for it and getting a + // parse error is the wrong lesson (mxcli-formula1 findings #8). + if ctx.CONFIGURATION() != nil { + if sl := ctx.STRING_LITERAL(); sl != nil { + stmt.Qualifier = unquoteString(sl.GetText()) + } + } + b.statements = append(b.statements, stmt) return } From 68d235fc40c4affe9c44c48528d95f174d49f1cc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:20:16 +0000 Subject: [PATCH 09/10] build(grammar): pin the ANTLR version where the failure happens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make build` needs an `antlr4` launcher, and the error when it is missing said only "install ANTLR4" — while the version is load-bearing. CI pins antlr4-tools 0.2.2 with generator 4.13.2 against the 4.13.1 runtime in go.mod, and a generator/runtime mismatch is a classic ANTLR failure mode that surfaces as a generated parser that will not compile. That pin lived only in the workflow YAML, where nobody hits it. The pin now lives in mdl/grammar/Makefile, the error message quotes it, and `make -C mdl/grammar bootstrap` does the pip install. `generate` also notes when ANTLR4_TOOLS_ANTLR_VERSION is unset, since the default is whatever antlr4-tools last downloaded. The README build-from-source recipe gains the bootstrap step and says what the build actually needs — network and a JVM, not only Go. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- README.md | 8 ++++++++ mdl/grammar/Makefile | 37 ++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 18de40a9e..5a96ca3d3 100644 --- a/README.md +++ b/README.md @@ -212,10 +212,18 @@ Or build from source (Go + Make — `make build` runs the ANTLR parser generatio ```bash git clone https://github.com/mendixlabs/mxcli.git cd mxcli +make -C mdl/grammar bootstrap # one-time: installs the pinned ANTLR generator +export ANTLR4_TOOLS_ANTLR_VERSION=4.13.2 make build # binary is at ./bin/mxcli ``` +The generator version is pinned and load-bearing: it must match the ANTLR +runtime in `go.mod`, or the generated parser will not compile. `antlr4-tools` +downloads the ANTLR jar on first run, so the build needs **network and a JVM**, +not only Go. Skip the bootstrap if you already have an `antlr4` launcher on +PATH (e.g. `brew install antlr4`). + > `go install …@latest` is not supported: the generated ANTLR parser isn't committed, so a module-source build fails. Use a pre-built binary or `make build`. ## Core Features diff --git a/mdl/grammar/Makefile b/mdl/grammar/Makefile index 17c751492..74e46e155 100644 --- a/mdl/grammar/Makefile +++ b/mdl/grammar/Makefile @@ -1,14 +1,15 @@ # Makefile for MDL grammar generation # # Usage: +# make bootstrap - Install the pinned ANTLR4 toolchain (pip + a JVM) # make generate - Generate Go parser from MDLLexer.g4 and MDLParser.g4 # make clean - Remove generated files # -# Prerequisites: -# - ANTLR4 must be installed (https://www.antlr.org/) -# - macOS: brew install antlr4 -# - Linux: apt install antlr4 (or download JAR manually) -# - Or use: pip install antlr4-tools +# Prerequisites: an `antlr4` launcher on PATH and a JVM. The generator version +# is PINNED and load-bearing: a generator/runtime mismatch is a classic ANTLR +# failure mode, and go.mod requires the runtime below. CI pins the same pair. +# `antlr4-tools` downloads the ANTLR jar on first run, so the build needs +# network and a JVM, not only Go. LEXER = MDLLexer.g4 PARSER = MDLParser.g4 @@ -27,18 +28,40 @@ DOMAIN_FILES = \ domains/MDLCatalog.g4 \ domains/MDLSettings.g4 +# Pinned toolchain. Keep in step with the CI workflows and with the +# github.com/antlr4-go/antlr/v4 requirement in go.mod. +ANTLR_TOOLS_VERSION = 0.2.2 +ANTLR_VERSION = 4.13.2 +ANTLR_RUNTIME_VERSION = 4.13.1 + # Find antlr4 command (try common locations) ANTLR4 := $(shell which antlr4 2>/dev/null || which antlr 2>/dev/null) -.PHONY: generate clean check-antlr +.PHONY: generate clean check-antlr bootstrap check-antlr: ifndef ANTLR4 - $(error ANTLR4 not found. Install with: brew install antlr4 (macOS) or pip install antlr4-tools) + $(error ANTLR4 not found. Run `make -C mdl/grammar bootstrap`, or install it yourself: \ + pip install 'antlr4-tools==$(ANTLR_TOOLS_VERSION)' && export ANTLR4_TOOLS_ANTLR_VERSION=$(ANTLR_VERSION) \ + (macOS alternative: brew install antlr4). The generator version is pinned to $(ANTLR_VERSION) \ + against runtime $(ANTLR_RUNTIME_VERSION) in go.mod — a mismatch produces a parser that does not compile) endif +# bootstrap installs the pinned generator. It needs pip and a JVM; antlr4-tools +# downloads antlr4-$(ANTLR_VERSION)-complete.jar on first run. +bootstrap: + pip install 'antlr4-tools==$(ANTLR_TOOLS_VERSION)' + @echo "" + @echo "Installed antlr4-tools $(ANTLR_TOOLS_VERSION)." + @echo "Export the pinned generator version before building:" + @echo " export ANTLR4_TOOLS_ANTLR_VERSION=$(ANTLR_VERSION)" + generate: check-antlr $(LEXER) $(PARSER) $(DOMAIN_FILES) @mkdir -p $(OUTPUT_DIR) + @if [ -z "$$ANTLR4_TOOLS_ANTLR_VERSION" ]; then \ + echo "Note: ANTLR4_TOOLS_ANTLR_VERSION is unset; this build pins $(ANTLR_VERSION)."; \ + echo " export ANTLR4_TOOLS_ANTLR_VERSION=$(ANTLR_VERSION)"; \ + fi $(ANTLR4) -Dlanguage=Go -no-visitor -package $(PACKAGE) -lib domains -o $(OUTPUT_DIR) $(LEXER) $(PARSER) @echo "Generated Go parser in $(OUTPUT_DIR)/" From 8e7bfcc40be1bd78951fb4767107caba91d653de Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:22:33 +0000 Subject: [PATCH 10/10] docs(new): say that step 6 links the local binary on Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 6 was documented as "downloads the correct mxcli binary", and on Linux it does not download: it hard-links the mxcli you ran into the project, sharing the inode. That is the right behaviour for a from-source build — the app folder gets your binary, not the nightly — but the help text described the opposite, which is what led one report to delete the linked binary before moving files around. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- cmd/mxcli/cmd_new.go | 2 +- docs-site/src/tutorial/installation.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/mxcli/cmd_new.go b/cmd/mxcli/cmd_new.go index 881a80309..f2a060e3d 100644 --- a/cmd/mxcli/cmd_new.go +++ b/cmd/mxcli/cmd_new.go @@ -26,7 +26,7 @@ This command performs the following steps: 3. Applies mxcli's default styling (--theme, see 'mxcli theme list') 4. Initializes AI tooling and devcontainer configuration (mxcli init) 5. Runs one build so generated sources are settled (--skip-build to skip) - 6. Downloads the correct mxcli binary for the devcontainer (linux) + 6. Links this mxcli into the project (or downloads a Linux build on macOS/Windows) Examples: mxcli new MyApp diff --git a/docs-site/src/tutorial/installation.md b/docs-site/src/tutorial/installation.md index ca65a6eef..402d3974c 100644 --- a/docs-site/src/tutorial/installation.md +++ b/docs-site/src/tutorial/installation.md @@ -76,7 +76,8 @@ This single command: 3. Sets up AI tooling (`.claude/`, skills, `AGENTS.md`) 4. Configures a Dev Container (`.devcontainer/`) 5. Runs one build, so the action stubs MxBuild regenerates are already settled -6. Downloads the correct Linux mxcli binary for the container +6. Puts an mxcli binary in the project — a hard link to the one you ran on Linux, + a downloaded Linux build on macOS/Windows (the container needs a Linux ELF) Open the resulting `MyApp/` folder in VS Code and click **"Reopen in Container"** — you're ready to go.