diff --git a/.claude/commands/mendix/lint.md b/.claude/commands/mendix/lint.md index e672b95816..6263a6896c 100644 --- a/.claude/commands/mendix/lint.md +++ b/.claude/commands/mendix/lint.md @@ -44,6 +44,7 @@ mxcli lint -p app.mpr --exclude System --exclude Administration | SEC001 | security | NoEntityAccessRules - Persistent entities need access rules | | SEC002 | security | WeakPasswordPolicy - Password minimum length should be 8+ | | SEC003 | security | DemoUsersActive - Demo users should be off at Production security | +| SEC010 | security | PageEntityNotReadable - A page data widget lists an entity the user roles that open the page cannot read; at Production it renders empty, silently (info when only row-scoped) | | CONV011 | performance | NoCommitInLoop - Commits inside loops (commit actions, or create/change with commit) cause N+1 issues | | CONV012 | quality | ExclusiveSplitCaption - Exclusive splits need meaningful captions | | CONV013 | quality | ErrorHandlingOnCalls - External calls (REST/WS/Java) need custom error handling | diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-db-provisioning-failure-invisible.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-db-provisioning-failure-invisible.json new file mode 100644 index 0000000000..f138374eaf --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-db-provisioning-failure-invisible.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "A session whose SessionStart hook failed to provision the app database (\"no local PostgreSQL superuser available\") looked like a healthy one: the hook runs `sh .claude/bootstrap-mxcli.sh || true` and the script ended in `exec ./mxcli run --local --setup --ensure-db`, so the failure was one line in hook output nobody reads and exit 0 (ChipCoV8/V9 bootstrap findings)", "cause": "`exec` handed the script's last word to mxcli, leaving the script no chance to react to the failure; `|| true` (correct — a hook must not block the session) then hid the status", "file": "`cmd/mxcli/init_hook.go` (`bootstrapScriptTemplate` tail); tests `cmd/mxcli/init_hook_failure_test.go`; `docs-site/src/tools/bootstrap-prompt.md`", "insight": "Keep `|| true` and move the loudness into the script: run without exec, tee the output to a log, keep the status in a file (a POSIX sh pipeline's status is tee's, and `cmd || echo $? >file` is the set -e-safe capture), then print an ERROR banner naming the last error line, the retry and `--db-type hsqldb`, and exit with that status. Test it by running the generated script under sh (dash here) with a stand-in mxcli whose `run` fails — the control (`run` succeeds) asserts no banner. Before the fix the failing case exited with the right status but printed no banner"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-script-defaults-to-nightly-for-release-builds.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-script-defaults-to-nightly-for-release-builds.json new file mode 100644 index 0000000000..a3715267f0 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-bootstrap-script-defaults-to-nightly-for-release-builds.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "The generated `.claude/bootstrap-mxcli.sh` defaulted `MXCLI_TAG` to `nightly` whatever mxcli wrote it, so a project initialised by a pinned release re-fetched the current nightly after every idle reap — a different mxcli from the one its tooling was written by (ChipCoV8/V9 bootstrap findings)", "cause": "`TAG=\"${MXCLI_TAG:-nightly}\"` was a literal in `bootstrapScriptTemplate`", "file": "`cmd/mxcli/init_hook.go` (`bootstrapDefaultTag`, `renderBootstrapScript`); tests `TestBootstrapDefaultTag`, `TestBootstrapScript_DefaultTagIsGeneratingRelease` in `cmd/mxcli/init_hook_test.go`; `docs-site/src/ide/syncing.md`", "insight": "Only a plain `vX.Y.Z` is a downloadable tag: `devcontainerReleaseTag`/`mxcliReleaseTag` strip a `-N-gHASH` suffix, which would map a dev build onto a release it is not — the bootstrap default must not do that, so it uses its own strict match and keeps nightly otherwise. The script is regenerated by `mxcli init` (not by `--sync-skills`), so the default is the release that last ran init"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-init-claudemd-titled-after-repo-folder.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-init-claudemd-titled-after-repo-folder.json new file mode 100644 index 0000000000..a83d45a4e3 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-init-claudemd-titled-after-repo-folder.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "`mxcli init` titled the generated CLAUDE.md `# Mendix Project: ` (and named the devcontainer after it): a cloud session clones into a folder named after the repository, so a seed repo produced `Mendix Project: seed` for an app called ChipCoV8 (ChipCoV8/V9 bootstrap findings)", "cause": "`projectName := filepath.Base(absDir)` in init, add-tool and the `init --sync-skills` doc refresh — the folder stood in for the app name although the .mpr file name was already in hand", "file": "`cmd/mxcli/init_claudemd.go` (`projectDisplayName`), `cmd/mxcli/init.go`, `cmd/mxcli/cmd_add_tool.go`, `cmd/mxcli/init_tooling_sync.go`; test `cmd/mxcli/init_appname_test.go`", "insight": "Every consumer of projectName is a display name (doc headings, devcontainer `name`, comments) — paths use absDir/mprFile — so one helper covers them all. The sync path matters as much as init: fixing init alone would have the session hook's `init --sync-skills` retitle the doc after the folder again. The `project.mpr` placeholder (no .mpr found) keeps the folder name. Control: with the call sites reverted to filepath.Base the init and sync tests fail"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-macos-jdk-resolves-to-usr-bin-java-stub.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-macos-jdk-resolves-to-usr-bin-java-stub.json new file mode 100644 index 0000000000..a1aea4d74e --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-macos-jdk-resolves-to-usr-bin-java-stub.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "On macOS, `mxcli new` for an 11.14+ (Java 25) project hung 10+ minutes in its final build step, running mxbuild with `--java-home=/usr --java-exe-path=/usr/bin/java`, Gradle at 100% CPU. `docker build`, `run --local` and `test --local` resolve the JDK the same way", "cause": "`resolveMacOSJavaHome` hard-coded `/usr/libexec/java_home -v 21`, so for a Java 25 project its answer always failed the major check and resolution fell through to `LookPath(\"java\")`. On macOS /usr/bin/java is a launcher stub, not a symlink: EvalSymlinks leaves it, the derived home is /usr, and `isJDK(\"/usr\", 25)` passes because the stub forwards `-version` to the default JDK", "file": "`cmd/mxcli/docker/detect.go` (`resolveJDKWith`, injectable `jdkEnv`, `isMacOSJavaStub`, `resolveMacOSJavaHome(major)`)", "insight": "A version probe through a forwarding launcher validates the launcher's target, not the path you derived from it — `isJDK` answered a question about a different directory than the one returned. The machine that exhibits this is one CI never runs, so the fix had to make the OS, java_home, LookPath and isJDK injectable; the fake models the stub's worst case (isJDK('/usr', n) true for every n). Revert proof: java_home -v 21 plus no /usr rejection returns /usr for a Java 25 project. Off macOS /usr is left alone — a Linux PATH java resolves through symlinks to /usr/lib/jvm/...", "refs": ["ChipCoV8 #3"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxbuild-runs-unbounded-new-hangs-forever.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxbuild-runs-unbounded-new-hangs-forever.json new file mode 100644 index 0000000000..d86c44a64c --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxbuild-runs-unbounded-new-hangs-forever.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "`mxcli new`'s final settle build (and `mxcli docker build`) could hang indefinitely — observed 10+ minutes on macOS with Gradle at 100% CPU and no output, and nothing ever reported or gave up", "cause": "`SettleGeneratedSources` and `mxbuildCmd` called `cmd.Run()` bare: no deadline, no detection of a build that had stopped printing, and killing only the direct child would not have helped since mxbuild is a wrapper whose JVM/Gradle grandchildren hold the output pipe", "file": "`cmd/mxcli/docker/mxbuild_watchdog.go` (`runWithWatchdog`, `WatchdogError`, `MXCLI_MXBUILD_TIMEOUT` default 15m, `MXCLI_MXBUILD_STALL_TIMEOUT` default 5m), wired in `settle.go` and `build.go` (`mxbuildCmd`)", "insight": "Bound the process TREE, not the child: the fake mxbuild in the test is `echo; sleep 60 & wait`, because a grandchild holding the pipe is exactly what makes `Wait` block after a plain Kill — reuse `setProcessGroup`/`killProcessGroup` and set `cmd.WaitDelay` as a backstop. The error carries the last output lines and the java home used, since a wrong --java-home was the cause the one time this was seen. Revert proof: `mxbuildCmd` with bare `cmd.Run()` leaves the hung fake running past the test's 20s guard. The trigger here was the /usr/bin/java stub (sibling finding); the watchdog is what turns the next unknown cause into an error instead of a hang", "refs": ["ChipCoV8 #6"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-syntax-teaches-spellings-check-flags-deprecated.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-syntax-teaches-spellings-check-flags-deprecated.json new file mode 100644 index 0000000000..c5678fc2ea --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-syntax-teaches-spellings-check-flags-deprecated.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "symptom": "`mxcli syntax` teaches spellings `check` warns on as deprecated, so an agent copying the reference gets MDL-DEPRnnn: `NOT NULL [ERROR 'msg']` (MDL-DEPR021, write `error message`), a named data grid `COLUMN name (` (MDL-DEPR005), `CREATE [OR REPLACE]` for microflow/nanoflow/layout/navigation (MDL-DEPR001), `MODEL accepts these keys` for `alter settings runtime` (MDL-DEPR555), the MOVE doctype list's QUEUE / REST CLIENT / ODATA CLIENT / ODATA SERVICE / MODEL, and `Date` as an attribute type (MDL-DEPR160, refused under mdl 1); the microflow workflow actions (`$Wf = call workflow M.WF ($Obj);`, get workflow data, workflow operation, lock/unlock …) had no topic at all", "cause": "TestExamplesParse proved each Example parses but never ran the validator, and the Syntax field (metasyntax) was checked only against hand lists of retired spellings — every deprecation added to the registry after a topic was written left that topic stale", "file": "`cmd/mxcli/syntax/features_{domain_model,misc,page,microflow,workflow}.go`; guards in `cmd/mxcli/syntax/example_parses_test.go` (deprecationErrors, container wrappers) and `cmd/mxcli/syntax/syntax_deprecations_test.go` (registry-driven fingerprints)", "insight": "Drive the guard from `mdl/deprecation`, not a hand list. Examples: run `executor.ValidateProgram` + `ApplyDeprecationPolicy(Error)` in EVERY context a block parses in, and host fragments in their containers (datagrid, dataview) because MDL-DEPR005 only fires on a column inside a grid. Syntax: diff each entry's Example against its CanonicalExample to get the changed tokens, then widen with context until the pattern holds a keyword, does not end on a name (`Ready comment` matched the prose \"a comment\"), and matches none of the known-clean corpus (all CanonicalExamples + every validated Example) — that corpus is what keeps `image collection` or `type:` from flagging canonical text. Drop `[`/`]` when lexing or `NOT NULL [ERROR 'msg']` escapes the pattern. Prose in `--` comments is not matched, so the MOVE doctype list and the RUNTIME prose had to be found by hand with `check`. Revert-proofed: restoring `ERROR 'msg'`, `COLUMN name (` or `head($L)` each fails with the code.", "refs": ["ChipCoV8 #9", "ChipCoV8 #12", "ChipCoV9 #4"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-test-exception-verdict-drops-error-message.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-test-exception-verdict-drops-error-message.json new file mode 100644 index 0000000000..97462aa4fc --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-11-mxcli-test-exception-verdict-drops-error-message.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-11", "raw": "| A `mxcli test` case whose body does `retrieve … where [id = '[%CurrentUser%]']` fails with only **FAIL:exception during execution** — nothing says what was raised | Two things at once. Runtime: tests run in `Core.createSystemContext()` (`endpoint.go`), a system session with no user, so the `[%CurrentUser%]` token cannot resolve and the retrieve throws; `[id = $currentUser]` only *seems* to work because `$currentUser` is empty there and matches nothing. mxcli: `rewriteBodyForVerdict` wrapped every CALL in an `ON ERROR` handler that set a **fixed** verdict text and discarded the error | `cmd/mxcli/testrunner/generator_endpoint.go` (`rewriteBodyForVerdict`), `.claude/skills/mendix/test-microflows/SKILL.md` | Concatenate the error: `'FAIL:exception during execution: ' + $latestError/Message`. No new measurement needed — `throwsHandler` already uses the same expression, measured at 0 errors on mxbuild 11.6.0 and 11.13.0. A generic catch-all verdict is the same defect class as a decorative assertion: it hides the one fact that diagnoses the failure. The skill now says plainly that tests have no current user and that user-scoped logic is tested by passing the user/account as a parameter. Not changed: the legacy after-startup runner's log-line `Exception during execution` (its runtime log already carries the stack trace). ChipCoV9 #5 |", "refs": ["ChipCoV9#5"]} diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-11-datagrid-unbound-attribute-column-tooltip-null.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-11-datagrid-unbound-attribute-column-tooltip-null.json new file mode 100644 index 0000000000..6e3355ed40 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-11-datagrid-unbound-attribute-column-tooltip-null.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-11", "symptom": "A Data grid 2 column with no attribute, no content widgets and no ShowContentAs (`column (Caption: '')`) still reports **CE0463** under `mx check --no-update-widgets` after its header is fixed; `fix widgets` changes only its `tooltip`, null -> empty Forms$ClientTemplate", "cause": "`detectObjectListItemKind` classifies a column as attribute-kind only when an attribute is BOUND, so an unbound column fell to `itemKindDefault`, which has no rule in `emptyClientTemplateRules` — its tooltip stayed null. update-widgets keys the convention on showContentAs, whose widget default is `attribute`", "file": "`mdl/backend/widgetobj/builder.go` (`buildObjectListItemBSON`, `effectiveShowContentAs`)", "insight": "Classify by the EFFECTIVE showContentAs (authored, else the widget's DefaultValue), not by whether an attribute is bound. Found only by diffing every column property against `fix widgets` output after the header fix (ChipCoV8 #11) — the header was the first of two differences on the same column, and checking only the reported property would have stopped there. Measured on 11.15.0: CE0463 gone; what remains is Mendix's own legitimate \"An attribute is required when 'Show' is set to 'Attribute'\", which the CE0463 had been masking", "refs": ["ChipCoV8 #11"], "ce": ["CE0463"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-datagrid-unnamed-column-empty-header-null-template.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-datagrid-unnamed-column-empty-header-null-template.json new file mode 100644 index 0000000000..815b2bbd71 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-datagrid-unnamed-column-empty-header-null-template.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-11", "symptom": "A Data grid 2 column with no caption (empty or absent), no attribute and **no name** — `column (Caption: '', ShowContentAs: customContent) { actionbutton … }`, `column (Caption: '')`, bare `column { … }` — passes `mxcli check`, but `mx check --no-update-widgets` reports **CE0463** on the grid. `fix widgets` clears it; re-exec rewrites the page and CE0463 returns", "cause": "`applyColumnHeaderFallback` returned early when there was no fallback text (no attribute, no column name), leaving the header an empty texttemplate, which `overlayItemValue` serializes as `TextTemplate: null`. `mx update-widgets` writes an empty `Forms$ClientTemplate` there (Fallback Items[3], Parameters[2], Template Items[3])", "file": "`mdl/executor/widget_engine.go` (`applyColumnHeaderFallback`)", "insight": "**The 2026-08-31 finding's \"verified, 0 errors for custom-content columns (empty + absent caption)\" was incomplete:** its tests and examples always passed a column NAME, so the name fallback hid the unnamed case. Fix: with no fallback, flag the header `EmptyTemplate` (in place, or appended) — scoped by the `hasHeaderSlot` gate, never all texttemplates (null vs empty differs per widget). Measured on 11.15.0 with update-widgets output as the reference: header shapes now byte-match it. A blind spot worth naming: a test fixture that always supplies the optional field (here the name) cannot see the branch where it is absent", "refs": ["ChipCoV8 #11", "ChipCoV9 #2"], "ce": ["CE0463"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-flow-verdict-skips-call-to-script-created-flow.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-flow-verdict-skips-call-to-script-created-flow.json new file mode 100644 index 0000000000..188b621831 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-11-flow-verdict-skips-call-to-script-created-flow.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-11", "symptom": "`check -p` printed \"no flow change exec would refuse\" (and exec's preflight said nothing) for a script whose `create or modify microflow` exec then refused with MDL-V1-REBUILD (\"replace $X: $X is still used by\") — after writing the statements before it. The trigger: the new body calls a flow an earlier statement of the same script creates", "cause": "CheckFlowVerdicts builds the declared flow against the stored project and skips any flow whose build fails as \"not predictable\"; a CALL MICROFLOW to a flow the script creates earlier fails microflowExists (\"not found in the project\"), so the verdict was silently skipped and the check line claimed full coverage", "file": "mdl/executor/validate_flow_verdicts.go (checkFlowVerdicts, flowModifyViolations), mdl/executor/cmd_microflows_builder.go (declaredFlow/scriptFlows overlay), cmd/mxcli/cmd_check.go, cmd/mxcli/exec_preflight.go", "insight": "A check-time prediction that skips what it cannot build must say so: a silent skip reads as a pass, and the skip set is exactly the multi-statement scripts where a half-applied exec hurts. Fix is an overlay of the script's own CREATE [OR MODIFY] MICROFLOW|NANOFLOW signatures in the builder (generalising the self-call `selfFlow` of #843), plus an Unpredicted list (FormatUnpredictedFlows) for whatever still does not build. Revert proof: stub the overlay lookup and the multi-statement case reports no violation, only the unpredicted note"} diff --git a/.claude/skills/fix-issue/findings/mdl-grammar/2026-10-11-qualified-association-hop-in-attribute-path.json b/.claude/skills/fix-issue/findings/mdl-grammar/2026-10-11-qualified-association-hop-in-attribute-path.json new file mode 100644 index 0000000000..cc74228fe7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-grammar/2026-10-11-qualified-association-hop-in-attribute-path.json @@ -0,0 +1 @@ +{"date": "2026-10-11", "area": "mdl/grammar", "symptom": "`column (attribute: FieldService.ServiceRequest_LithoSystem/SerialNumber)` in a Data grid 2 failed `mxcli check` with MDL-WIDGET34 (printing the value as `&{…}`, the column as \"column ``\", and advising to wrap it in a data view although it sat in a grid) plus MDL-WIDGET42 \"cannot hold the expression\"; the bare `ServiceRequest_LithoSystem/SerialNumber` passed. XPath and data view sources name the same association module-qualified, so the qualified spelling is the natural one.", "cause": "`attributePathV3` (MDLPage.g4) took only plain identifiers between slashes, so `Module.Assoc/Attr` missed `ATTRIBUTE COLON attributePathV3` and fell through to the generic `keyword COLON expression` alternative, parsing as the division `Module.Assoc / Attr` -> ast.WidgetExpressionV3. The executor already accepted qualified hops (resolveAssociationPathIn keeps a dotted name). The WIDGET34 message formatted the non-string value with %v and assumed a named widget outside any data container.", "fix": "Grammar: `attributePathV3 : (attributePathHopV3 SLASH)* attributePathSegmentV3`, a hop optionally `Module.Assoc`; the final segment stays bare so `M.E.A` (no slash) is not a path. buildAttributePathV3 unquotes each dotted part. Message: nonStringAttributeText returns WidgetExpressionV3.Text; nonPathBindingProblem names an unnamed widget by caption, suggests `Assoc/Attr or Module.Assoc/Attr` for a value with `/`, and drops the wrap-in-a-data-view advice inside a data container.", "insight": "When a value reaches a checker as an expression where a path was meant, look at the grammar alternative it fell out of before the checker: the generic `keyword: expression` fallback silently absorbs every near-miss of a specific property rule, and the checker then reports the symptom (\"cannot hold the expression\"), not the cause. The builder side needed no change — a builder test pinning qualified == bare AttributeRef guards it. Describe still prints the short form.", "issue": "ChipCoV8 #10", "file": "mdl/grammar/domains/MDLPage.g4 (attributePathV3), mdl/visitor/visitor_page_v3.go (buildAttributePathV3), mdl/executor/cmd_pages_input_binding_context.go (nonPathBindingProblem, nonStringAttributeText)", "test": "mdl/visitor/visitor_page_qualified_attribute_path_test.go, mdl/executor/cmd_pages_builder_qualified_assoc_path_test.go, mdl/executor/cmd_pages_input_binding_context_test.go TestValidateInputBindingNonPathValueMessage"} diff --git a/.claude/skills/fix-issue/findings/mdl-other/2026-10-12-sec010-specialization-rule-without-generalization-members.json b/.claude/skills/fix-issue/findings/mdl-other/2026-10-12-sec010-specialization-rule-without-generalization-members.json new file mode 100644 index 0000000000..a90e68587f --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-other/2026-10-12-sec010-specialization-rule-without-generalization-members.json @@ -0,0 +1 @@ +{"area": "mdl/linter", "date": "2026-10-12", "symptom": "SEC010 stayed silent on a list over `System.User` for a ServiceManager, whose browser showed every row with `Name` blank except their own (`[\"\", \"\", \"demo_manager\", \"\"]`) — the very case the rule was written for (ChipCoV9 FINDINGS, re-check on main)", "cause": "The rule counted any entity-level read on a specialization as reading the generalization. The real Mendix 11.15 Administration module grants Administration.User `read (FullName, Email)` on Account UNCONSTRAINED: the rows come through, but no `System.User` member is in that rule, so the fields are blank. The unit fixture had modelled that rule as row-scoped, so the tests agreed with the code and not with the platform", "file": "`mdl/linter/rules/page_entity_read_access.go` (`entityAccessModel.level`, `memberOwners`); tests `TestPageEntityReadAccess_SystemUserMembersNotGrantedBySpecialization`, `..._SystemUserManageAllRolesReadsMembers`", "insight": "A specialization's rule fills ROWS of a list over the generalization, and FIELDS only for members the rule grants — MEMBER_READ rows are qualified by the declaring entity, so the owner prefix tells. Measured on a scratch copy (run --local, HSQLDB, playwright): an Administrator's Account rule grants no System.User member either, yet every name showed — System.User's own rule also opens users whose roles the user role may manage (ManageableRoles FieldEngineer → demo_engineer's name appeared). Build a lint fixture from `show access` / the catalog of a real project, never from the rule as you remember it"} diff --git a/.claude/skills/mendix/assess-quality/SKILL.md b/.claude/skills/mendix/assess-quality/SKILL.md index e3ff4b8522..e9ca03e1a0 100644 --- a/.claude/skills/mendix/assess-quality/SKILL.md +++ b/.claude/skills/mendix/assess-quality/SKILL.md @@ -12,14 +12,14 @@ description: "Audit a Mendix project against best practices for naming, security |---|---| | 29-36 | When to Use This Skill | | 37-96 | Assessment Workflow | -| 97-351 | Quality Guidelines Reference | -| 352-362 | Assessment Report Template | -| 363-375 | Executive Summary | -| 376-386 | project Metrics | -| 387-399 | Automated Findings | -| 400-419 | Manual Review Findings | -| 420-435 | Recommendations | -| 436-446 | Sources | +| 97-352 | Quality Guidelines Reference | +| 353-363 | Assessment Report Template | +| 364-376 | Executive Summary | +| 377-387 | project Metrics | +| 388-400 | Automated Findings | +| 401-420 | Manual Review Findings | +| 421-436 | Recommendations | +| 437-447 | Sources | # Assess Mendix Project Quality @@ -152,13 +152,14 @@ After reviewing automated results, assess the following areas manually. The guid ### C. Security -**Enforced by rules:** SEC001-SEC009, CONV006, CONV007, CONV008, MPR007 +**Enforced by rules:** SEC001-SEC010, CONV006, CONV007, CONV008, MPR007 #### C.1 Entity Access | Guideline | Rule | Priority | |-----------|------|----------| | All persistent entities must have access rules | SEC001 | Critical | +| Every role that opens a page can read the entities its grids, galleries and combo boxes list (else it renders empty at Production) | SEC010 | High | | Do not grant Create or Delete rights on entity access rules | CONV006 | High | | All entity access rules should have XPath constraints | CONV007 | High | | Set default access rights to None | Manual check | High | @@ -438,7 +439,7 @@ Overall Score: [X]/100 (from `mxcli report`) This skill incorporates guidelines from: - **Conventions.pdf** — Squad Apps internal best practices (14 categories, 80+ guidelines) - **CONV001-CONV017 lint rules** — Automated checks derived from Conventions.pdf -- **MPR001-MPR007, SEC001-SEC009** — Built-in linter rules +- **MPR001-MPR007, SEC001-SEC010** — Built-in linter rules - **ARCH, DESIGN, QUAL series** — Starlark architecture/quality rules - **Mendix Performance Best Practices** — Official Mendix documentation on performance optimization - **Mendix Security Best Practices** — Official Mendix documentation on security configuration diff --git a/.claude/skills/mendix/bootstrap-app/SKILL.md b/.claude/skills/mendix/bootstrap-app/SKILL.md index 6e58fb6fb0..4e09151e68 100644 --- a/.claude/skills/mendix/bootstrap-app/SKILL.md +++ b/.claude/skills/mendix/bootstrap-app/SKILL.md @@ -77,7 +77,7 @@ it is building. `ledger` (light, dense, data-heavy), `console` (dark), or `none` for stock Atlas. Default `signal`. 7. **Mendix version.** Default to whatever the session environment already provides — - see below — and otherwise the newest version on the CDN (today `11.14.0`). + see below — and otherwise the newest version on the CDN. 8. **Do you have requirements to work from?** A specification document, a prototype, a wireframe, a long description — anything that is the source of truth but is not in this repo. **Default: yes, record them.** If they say yes, @@ -109,12 +109,12 @@ the CDN as below and accept the download. **Otherwise take the newest version on the CDN.** Everything mxcli does starts with downloading MxBuild, so "supported" means "on the CDN", and `run --local` needs the -runtime tarball as well. At the time of writing the newest is **11.14.0** (11.15.0 is -not published). Do not trust that number — it ages. Confirm the one you land on, and -walk backwards a minor if it 404s: +runtime tarball as well. A remembered version number ages, so confirm it. Probe the CDN: start from the newest version you know of, try the +next minor up until one 404s, and take the highest that answers. If the one you +start from 404s, walk backwards a minor: ```bash -V=11.14.0 +V=11.N.0 # the candidate curl -sI -o /dev/null -w '%{http_code}\n' https://cdn.mendix.com/runtime/mxbuild-$V.tar.gz curl -sI -o /dev/null -w '%{http_code}\n' https://cdn.mendix.com/runtime/mendix-$V.tar.gz ``` diff --git a/.claude/skills/mendix/manage-security/SKILL.md b/.claude/skills/mendix/manage-security/SKILL.md index 4225948cc1..71a8ac1199 100644 --- a/.claude/skills/mendix/manage-security/SKILL.md +++ b/.claude/skills/mendix/manage-security/SKILL.md @@ -12,12 +12,12 @@ description: "Configure Mendix security in MDL — module and user roles, access |---|---| | 27-35 | When to Use This Skill | | 36-43 | Security Concepts | -| 44-110 | The System-module ceiling — decide the role model around this FIRST | -| 111-540 | Syntax Reference | -| 541-555 | Starlark Lint Rule APIs | -| 556-608 | Common Workflow: Setting Up Module Security | -| 609-618 | Common Mistakes | -| 619-628 | Validation | +| 44-120 | The System-module ceiling — decide the role model around this FIRST | +| 121-550 | Syntax Reference | +| 551-565 | Starlark Lint Rule APIs | +| 566-618 | Common Workflow: Setting Up Module Security | +| 619-628 | Common Mistakes | +| 629-638 | Validation | # Security Management Skill @@ -55,7 +55,7 @@ Every consequence is **silent** — the page renders, the data is simply missing | What you build | What a non-Administrator sees | |---|---| -| A combo box over `System.User` (e.g. "pick a technician") | **The current user only** | +| A combo box or grid over `System.User` (e.g. "pick a technician") | **Every user, name blank** — except their own, and those whose roles their user role may manage | | A grid over `System.Workflow` / `System.WorkflowUserTask` | **Empty** | | Either of those, re-sourced from a **microflow** | **Every row present, every field blank** | @@ -79,8 +79,18 @@ the *same* `System.User` retrieve, opened by two users: Both lists hold two rows for both users, so the microflow really did carry the rows past entity access. Only A loses the values, and it loses them **per object**: -`System.User`'s own rule grants read where `[id = '[%CurrentUser%]']`, which is why -a user picker shows you yourself and nobody else rather than showing nothing. +`System.User`'s own rule grants read where `[id = '[%CurrentUser%]']` — or where the +user's roles are ones your user role may manage — which is why a user picker shows +your own name and blanks rather than nothing. + +The same happens **without** a microflow. A database list over `System.User` gets its +rows from `Administration.Account`'s rule (unconstrained for `Administration.User`), +but that rule grants only Account's own members (`FullName`, `Email`), not +`System.User.Name`. Measured on 11.15 (ChipCoV9): a ServiceManager saw +`["", "", "demo_manager", ""]`; given `ManageableRoles: (FieldEngineer)` it also +saw `demo_engineer`; an Administrator (Manage All Roles) saw every name. +`mxcli lint` reports this as SEC010. Bind the list to `Administration.Account` and +show `FullName` instead. `System.WorkflowUserTask` has no such escape hatch for an ordinary role, so a workflow inbox built this way comes out *entirely* blank — the reported case diff --git a/.claude/skills/mendix/test-microflows/SKILL.md b/.claude/skills/mendix/test-microflows/SKILL.md index d175e69166..fed89b612b 100644 --- a/.claude/skills/mendix/test-microflows/SKILL.md +++ b/.claude/skills/mendix/test-microflows/SKILL.md @@ -12,18 +12,18 @@ description: "Write and run MDL-based microflow tests with `mxcli test` — anno |---|---| | 33-39 | Reference files | | 40-50 | When to Use This Skill | -| 51-178 | Prerequisites | -| 179-221 | Test File Formats | -| 222-237 | string Operations | -| 238-261 | Running Tests | -| 262-433 | How It Works | -| 434-460 | Check a test file before you run it | -| 461-567 | Writing Good Tests | -| 568-582 | Test File Organization | -| 583-593 | Interpreting Failures | -| 594-615 | CI Integration | -| 616-622 | Related Skills | -| 623-625 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | +| 51-179 | Prerequisites | +| 180-222 | Test File Formats | +| 223-238 | string Operations | +| 239-262 | Running Tests | +| 263-450 | How It Works | +| 451-477 | Check a test file before you run it | +| 478-584 | Writing Good Tests | +| 585-599 | Test File Organization | +| 600-612 | Interpreting Failures | +| 613-634 | CI Integration | +| 635-641 | Related Skills | +| 642-644 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | # Test Microflows Skill @@ -63,7 +63,8 @@ mxcli test tests/ -p app.mpr --local # no daemon needed mxcli test tests/ -p app.mpr # Docker ``` -`--local` uses its own ports (app 8081, admin 8091) and its own `_test` +`--local` uses its own ports (app 8081, admin 8091, serve 6544 — move them with +`--app-port`/`--admin-port`/`--serve-port`) and its own `_test` database, so a `mxcli run --local` dev loop can keep serving the same project while the tests run — the tests never write into the database you are looking at in the browser. The database is created on first use. @@ -384,6 +385,22 @@ Three things to know before reaching for it: | `--local --watch` | ~30s once, then ~2s | `_test` | yes | | `--attach` | none | the running app's | no | +#### Tests run as the system, with no current user + +Every test runs in a **system context** — there is no user session behind it: + +- An XPath token `[%CurrentUser%]` (e.g. `retrieve … where [id = '[%CurrentUser%]']`) + **throws**: the runtime cannot resolve it, and the test fails with + `exception during execution: `. +- `$currentUser` is **empty**. `[id = $currentUser]` does not throw, it just matches + nothing — a test that "passes" with it has tested nothing user-specific. +- Entity access rules and XPath constraints for user roles do not apply. + +To test user-scoped logic, take the user (or the account) as a **parameter** of the +microflow that does the work, and keep `[%CurrentUser%]`/`$currentUser` in a thin +wrapper that only passes it on. The test creates or retrieves a user and calls the +parameterised microflow directly. + #### Security of the endpoint It executes microflows under a system context, so it is gated four ways: @@ -584,10 +601,12 @@ tests/ | Failure | Cause | Fix | |---------|-------|-----| -| `Exception during execution` | Microflow threw a runtime error | Check BSON structure, entity references, attribute types | +| `exception during execution: ` | Microflow threw a runtime error; the runtime's message follows the colon | Read the message. Check entity references, attribute types | +| `exception during execution: …` from a retrieve using `[%CurrentUser%]` | Tests run in a system context with no current user, so the token cannot resolve (and `$currentUser` is empty) | Pass the user/account to the microflow as a parameter and test that microflow | | `Expected $result = 'X' but got 'Y'` | Wrong return value | Fix microflow logic | | `Test was not executed` | Runtime crashed before reaching it | Check earlier test failures or runtime logs | | `after startup microflow should return a boolean` | Generated runner has wrong return type | Report as bug in mxcli | +| `port 8081 (app) is already in use — pass --app-port ` | Something else (often an editor helper) holds a `--local` port; probed before the project is touched | Re-run with `--app-port 9081` (or `--admin-port`/`--serve-port` for the port named); kill the holder only if the message says it is a leftover mxcli run | --- diff --git a/cmd/mxcli/cmd_add_tool.go b/cmd/mxcli/cmd_add_tool.go index 7edd770955..ed6fe27a28 100644 --- a/cmd/mxcli/cmd_add_tool.go +++ b/cmd/mxcli/cmd_add_tool.go @@ -97,7 +97,7 @@ Supported Tools: if mprFile == "" { mprFile = "project.mpr" // Default if not found } - projectName := filepath.Base(absDir) + projectName := projectDisplayName(absDir, findMprFile(absDir)) fmt.Printf("Adding %s support to: %s\n", toolConfig.Name, absDir) diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index ef2a61d095..e1d45cf446 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -447,7 +447,8 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { // run the verdict exec and diff run, so the three agree. projectViolations := exec.CheckEntityMemberDrops(prog) projectViolations = append(projectViolations, exec.TypeCheckProgram(prog)...) - projectViolations = append(projectViolations, exec.CheckFlowVerdicts(prog)...) + flowReport := exec.CheckFlowVerdictsReport(prog) + projectViolations = append(projectViolations, flowReport.Violations...) // MDL-SEC21 (MxBuild CE0106): a flow this script leaves with no allowed // role while a page, snippet, nanoflow, menu or navigation uses it. The // reported shape was drop + create in separate runs, which loses the @@ -472,7 +473,17 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { return finish(1) } } else if !isStructured { - fmt.Printf("✓ Expression types OK, no unstated member drops, no flow change exec would refuse, no used flow left without access\n") + flows := "no flow change exec would refuse" + if len(flowReport.Unpredicted) > 0 { + flows = "no predicted flow change exec would refuse" + } + fmt.Printf("✓ Expression types OK, no unstated member drops, %s, no used flow left without access\n", flows) + } + // A flow change whose verdict depends on what earlier statements + // create is not predicted; say so rather than let the line above + // read as covering it. + if !isStructured && len(flowReport.Unpredicted) > 0 { + fmt.Fprint(os.Stderr, executor.FormatUnpredictedFlows(flowReport.Unpredicted)) } } diff --git a/cmd/mxcli/cmd_describe.go b/cmd/mxcli/cmd_describe.go index 80ab7e6298..03d5bbfe6f 100644 --- a/cmd/mxcli/cmd_describe.go +++ b/cmd/mxcli/cmd_describe.go @@ -72,6 +72,7 @@ Types: demouser Describe a demo user navigation Describe a navigation profile systemoverview Module dependency graph (requires --format elk) + domainmodel A module's domain model as a diagram (--format svg or elk; the name is the module) Example: mxcli describe -p app.mpr MyModule.Customer # type auto-detected @@ -89,6 +90,7 @@ Example: mxcli describe -p app.mpr settings Settings mxcli describe -p app.mpr navigation Responsive mxcli describe -p app.mpr --format elk systemoverview SystemOverview + mxcli describe -p app.mpr --format svg domainmodel MyModule mxcli describe -p app.mpr agent MyModule.MyAgent mxcli describe -p app.mpr model MyModule.GPT4 mxcli describe -p app.mpr knowledge base MyModule.ProductDocs @@ -161,6 +163,17 @@ Example: os.Exit(1) } return + } else if format == "svg" { + upper := objectType + if upper == "DOMAINMODEL" || upper == "DOMAIN MODEL" || (upper == "ENTITY" && !strings.Contains(name, ".")) { + if err := exec.DomainModelSVG(name); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } + return + } + fmt.Fprintf(os.Stderr, "SVG format not supported for type: %s (supported: domainmodel )\n", typeArg) + os.Exit(1) } else if format == "elk" { upper := objectType if upper == "SYSTEMOVERVIEW" { @@ -200,9 +213,9 @@ Example: return } - // SYSTEMOVERVIEW requires elk format + // SYSTEMOVERVIEW and DOMAINMODEL are diagrams with no MDL form if mdlCmd == "" { - fmt.Fprintf(os.Stderr, "Type %s requires --format elk\n", args[0]) + fmt.Fprintf(os.Stderr, "Type %s is a diagram: use --format elk (or --format svg for a domain model)\n", args[0]) os.Exit(1) } @@ -318,8 +331,8 @@ func describeMDLCommand(objectType, name string) (mdlCmd string, ok bool) { return fmt.Sprintf("DESCRIBE CONSUMED MCP SERVICE %s", name), true case "DATATRANSFORMER", "DATA TRANSFORMER": return fmt.Sprintf("DESCRIBE DATA TRANSFORMER %s", name), true - case "SYSTEMOVERVIEW": - return "", true // handled directly by format-specific path + case "SYSTEMOVERVIEW", "DOMAINMODEL", "DOMAIN MODEL": + return "", true // diagrams only: handled by the --format elk / svg paths } return "", false } diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go index a74b6a963a..fb145b0bcb 100644 --- a/cmd/mxcli/cmd_lint.go +++ b/cmd/mxcli/cmd_lint.go @@ -33,6 +33,7 @@ Built-in rules check for: - Entity access rules (SEC001) - persistent entities need access rules - Password policy (SEC002) - password minimum length should be 8+ - Demo users (SEC003) - demo users should be off at Production security + - Unreadable page data (SEC010) - a data widget's entity must be readable by the roles that open its page (at Production it renders empty) - Un-describable branch structure (MDL-FLOW01) - decision branches that do not nest, so DESCRIBE cannot render them faithfully Bundled Starlark rules (in .claude/lint-rules/): @@ -424,6 +425,7 @@ func builtinLintRules() []linter.Rule { rules.NewNoEntityAccessRulesRule(), rules.NewWeakPasswordPolicyRule(), rules.NewDemoUsersActiveRule(), + rules.NewPageEntityReadAccessRule(), // SEC010 - full catalog (widgets, permissions) rules.NewOverlappingActivitiesRule(), // MPR008 - requires BSON inspection rules.NewLoopChildContainmentRule(), // MPR011 - requires BSON inspection rules.NewNoCommitInLoopRule(), // CONV011-CONV014 - require BSON inspection diff --git a/cmd/mxcli/cmd_test_run.go b/cmd/mxcli/cmd_test_run.go index 0060609d4c..567677d763 100644 --- a/cmd/mxcli/cmd_test_run.go +++ b/cmd/mxcli/cmd_test_run.go @@ -121,6 +121,9 @@ Examples: # ...on the built-in file database, with no PostgreSQL on the machine mxcli test tests/ -p app.mpr --local --db-type hsqldb + # ...on other ports, when 8081/8091/6544 are taken (e.g. by an editor helper) + mxcli test tests/ -p app.mpr --local --app-port 9081 --admin-port 9091 --serve-port 7544 + # Keep the runtime warm and re-run on every change mxcli test tests/ -p app.mpr --local --watch @@ -206,6 +209,7 @@ Examples: Stdout: os.Stdout, Stderr: os.Stderr, } + applyTestPortFlags(cmd, &opts) // Only a --local run boots an app of its own, so only it decides which // constants that app runs with. --attach inherits the constants of the app @@ -295,3 +299,18 @@ func resolveTestPaths(paths []string, projectPath string) []string { } return out } + +// registerTestPortFlags adds the port flags `run --local` has, with the test +// boot's own defaults: a test run sits beside a dev loop on 8080/8090/6543. +func registerTestPortFlags(cmd *cobra.Command) { + cmd.Flags().Int("app-port", 0, "With --local, HTTP port for the test app (default 8081)") + cmd.Flags().Int("admin-port", 0, "With --local, M2EE admin API port for the test app (default 8091)") + cmd.Flags().Int("serve-port", 0, "With --local, mxbuild --serve port for the test build (default 6544)") +} + +// applyTestPortFlags copies the port flags onto opts; zero leaves the default. +func applyTestPortFlags(cmd *cobra.Command, opts *testrunner.RunOptions) { + opts.AppPort, _ = cmd.Flags().GetInt("app-port") + opts.AdminPort, _ = cmd.Flags().GetInt("admin-port") + opts.ServePort, _ = cmd.Flags().GetInt("serve-port") +} diff --git a/cmd/mxcli/cmd_test_run_ports_test.go b/cmd/mxcli/cmd_test_run_ports_test.go new file mode 100644 index 0000000000..3f3d7d3ca0 --- /dev/null +++ b/cmd/mxcli/cmd_test_run_ports_test.go @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/testrunner" + "github.com/spf13/cobra" +) + +// ChipCoV8 #14: `test --local` had no way to move off 8081/8091/6544. +func TestTestPortFlags_ReachRunOptions(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + registerTestPortFlags(cmd) + if err := cmd.ParseFlags([]string{"--app-port", "9081", "--admin-port=9091", "--serve-port", "7544"}); err != nil { + t.Fatal(err) + } + var opts testrunner.RunOptions + applyTestPortFlags(cmd, &opts) + if opts.AppPort != 9081 || opts.AdminPort != 9091 || opts.ServePort != 7544 { + t.Errorf("ports = %d/%d/%d, want 9081/9091/7544", opts.AppPort, opts.AdminPort, opts.ServePort) + } + + // Control: unset flags leave the options at zero, which the runner resolves + // to its defaults. + cmd = &cobra.Command{Use: "test"} + registerTestPortFlags(cmd) + if err := cmd.ParseFlags(nil); err != nil { + t.Fatal(err) + } + opts = testrunner.RunOptions{} + applyTestPortFlags(cmd, &opts) + if opts.AppPort != 0 || opts.AdminPort != 0 || opts.ServePort != 0 { + t.Errorf("unset flags gave ports %d/%d/%d, want 0/0/0", opts.AppPort, opts.AdminPort, opts.ServePort) + } +} + +func TestTestCommandRegistersPortFlags(t *testing.T) { + for _, name := range []string{"app-port", "admin-port", "serve-port"} { + if testRunCmd.Flags().Lookup(name) == nil { + t.Errorf("mxcli test has no --%s flag", name) + } + } +} diff --git a/cmd/mxcli/docker/build.go b/cmd/mxcli/docker/build.go index 8a9848c5bd..6f48ea22dd 100644 --- a/cmd/mxcli/docker/build.go +++ b/cmd/mxcli/docker/build.go @@ -65,7 +65,7 @@ var mxbuildCmd = func(mxbuildPath, javaHome, outputDir, mprPath string, w, stder cmd.Stdout = w cmd.Stderr = stderr PrepareMxCommand(cmd) - return cmd.Run() + return runWithWatchdog(cmd, mxbuildWatchdogLimits(), javaHome) } // buildOnCopy runs the steps of a build that read the model — mx update-widgets, diff --git a/cmd/mxcli/docker/detect.go b/cmd/mxcli/docker/detect.go index 9bba775481..ce2654f3cf 100644 --- a/cmd/mxcli/docker/detect.go +++ b/cmd/mxcli/docker/detect.go @@ -288,44 +288,95 @@ func mxbuildSearchPaths() []string { // constant: Mendix 11.14's blank app asks for Java 25, matching Studio Pro's own // support for it, while every version up to 11.13 asks for 21. Resolving 21 // unconditionally is what made an 11.14 project impossible to build or boot. -func resolveJDK(major int) (string, error) { +func resolveJDK(major int) (string, error) { return resolveJDKWith(hostJDKEnv(), major) } + +// jdkEnv is everything resolveJDK asks of the machine, injectable so the macOS +// path is testable from a Linux runner — the /usr/bin/java stub that hung +// `mxcli new` exists on no machine CI runs on. +type jdkEnv struct { + goos string + getenv func(string) string + macJavaHome func(major int) (string, error) // /usr/libexec/java_home -v + lookPath func(string) (string, error) + evalSymlinks func(string) (string, error) + isJDK func(home string, major int) bool + glob func(string) ([]string, error) + searchPaths func(major int) []string + versionOf func(home string) string +} + +func hostJDKEnv() jdkEnv { + return jdkEnv{ + goos: runtime.GOOS, + getenv: os.Getenv, + macJavaHome: resolveMacOSJavaHome, + lookPath: exec.LookPath, + evalSymlinks: filepath.EvalSymlinks, + isJDK: isJDK, + glob: filepath.Glob, + searchPaths: jdkSearchPaths, + versionOf: javaVersionString, + } +} + +// isMacOSJavaStub reports whether home is what the PATH fallback derives from +// macOS's /usr/bin/java. That file is a launcher stub, not a symlink, so +// EvalSymlinks leaves it alone and the derived home is /usr. Worse, isJDK +// cannot tell: the stub forwards `-version` to the default JDK, so /usr +// "passes" for any major that happens to be installed, and MxBuild handed +// --java-home=/usr spins Gradle at 100% CPU indefinitely (ChipCoV8 #3). +func isMacOSJavaStub(home string) bool { + return filepath.Clean(home) == "/usr" +} + +func resolveJDKWith(e jdkEnv, major int) (string, error) { major = javaMajorOrDefault(major) + darwin := e.goos == "darwin" // Try JAVA_HOME - if javaHome := os.Getenv("JAVA_HOME"); javaHome != "" { - if isJDK(javaHome, major) { + if javaHome := e.getenv("JAVA_HOME"); javaHome != "" { + if !(darwin && isMacOSJavaStub(javaHome)) && e.isJDK(javaHome, major) { return javaHome, nil } // JAVA_HOME set but wrong version — continue searching } - // On macOS, use /usr/libexec/java_home to find the real JDK path. - // The /usr/bin/java shim is not a symlink, so EvalSymlinks won't resolve - // it to the actual JDK, and MxBuild needs the real installation path. - if runtime.GOOS == "darwin" { - if javaHome, err := resolveMacOSJavaHome(); err == nil && isJDK(javaHome, major) { + // On macOS, use /usr/libexec/java_home to find the real JDK path — for the + // PROJECT's major, not a fixed 21 (a fixed 21 always failed the check below + // for an 11.14+ project and fell through to the /usr/bin/java stub). + var macErr error + if darwin { + javaHome, err := e.macJavaHome(major) + switch { + case err != nil: + macErr = err + case isMacOSJavaStub(javaHome): + macErr = fmt.Errorf("java_home answered %s", javaHome) + case e.isJDK(javaHome, major): return javaHome, nil + default: + macErr = fmt.Errorf("java_home answered %s, which is not a JDK %d", javaHome, major) } } // Try java in PATH - if javaPath, err := exec.LookPath("java"); err == nil { + if javaPath, err := e.lookPath("java"); err == nil { // Resolve symlinks to find the actual installation - resolved, err := filepath.EvalSymlinks(javaPath) - if err == nil { + if resolved, err := e.evalSymlinks(javaPath); err == nil { javaPath = resolved } // java binary is typically at /bin/java javaHome := filepath.Dir(filepath.Dir(javaPath)) - if isJDK(javaHome, major) { + if !(darwin && isMacOSJavaStub(javaHome)) && e.isJDK(javaHome, major) { return javaHome, nil } } // Try OS-specific known locations - for _, pattern := range jdkSearchPaths(major) { - matches, _ := filepath.Glob(pattern) + searched := e.searchPaths(major) + for _, pattern := range searched { + matches, _ := e.glob(pattern) for _, m := range matches { - if isJDK(m, major) { + if e.isJDK(m, major) { return m, nil } } @@ -334,15 +385,25 @@ func resolveJDK(major int) (string, error) { // Name what was searched. "JDK 21 not found" alone sent a Windows user // hunting through mxcli's source for the detection logic; the list makes it // obvious whether their JDK simply sits somewhere unlisted. - searched := jdkSearchPaths(major) msg := fmt.Sprintf("JDK %d not found", major) - if jh := os.Getenv("JAVA_HOME"); jh != "" { - msg += fmt.Sprintf("\n JAVA_HOME is set to %s but is not a JDK %d (java -version reports %s)", jh, major, javaVersionString(jh)) + if jh := e.getenv("JAVA_HOME"); jh != "" { + msg += fmt.Sprintf("\n JAVA_HOME is set to %s but is not a JDK %d (java -version reports %s)", jh, major, e.versionOf(jh)) + } + if darwin { + msg += fmt.Sprintf("\n /usr/libexec/java_home -v %d found none", major) + if macErr != nil { + msg += fmt.Sprintf(" (%v)", firstLine(strings.TrimSpace(macErr.Error()))) + } + msg += "; /usr/bin/java is the macOS launcher stub, not a JDK home, so it is not used." } if len(searched) > 0 { msg += "\n Searched: " + strings.Join(searched, ", ") } msg += fmt.Sprintf("\n Install Eclipse Temurin JDK %d, or set JAVA_HOME to one.", major) + if darwin { + msg += fmt.Sprintf("\n On macOS: brew install --cask temurin@%d, then check with /usr/libexec/java_home -v %d"+ + "\n (or export JAVA_HOME=$(/usr/libexec/java_home -v %d)).", major, major, major) + } // Which Java a project needs is a MODEL setting, so a user staring at this // has a second way out that a bare "install a JDK" hides: build for a release // they already have. Only say so when the requirement is not the default, @@ -355,9 +416,10 @@ func resolveJDK(major int) (string, error) { return "", fmt.Errorf("%s", msg) } -// resolveMacOSJavaHome uses /usr/libexec/java_home to find a JDK 21 on macOS. -func resolveMacOSJavaHome() (string, error) { - out, err := exec.Command("/usr/libexec/java_home", "-v", "21").Output() +// resolveMacOSJavaHome asks /usr/libexec/java_home for a JDK of the given major +// on macOS. java_home exits non-zero when none is installed. +func resolveMacOSJavaHome(major int) (string, error) { + out, err := exec.Command("/usr/libexec/java_home", "-v", strconv.Itoa(major)).Output() if err != nil { return "", err } diff --git a/cmd/mxcli/docker/detect_macos_java_test.go b/cmd/mxcli/docker/detect_macos_java_test.go new file mode 100644 index 0000000000..85738678dc --- /dev/null +++ b/cmd/mxcli/docker/detect_macos_java_test.go @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "errors" + "strconv" + "strings" + "testing" +) + +// fakeMacJDKEnv models a Mac with a Java 25 project in view: /usr/bin/java is +// the launcher stub (not a symlink), which forwards `-version` to whatever JDK +// is default — so isJDK("/usr", n) answers yes for any installed major. That +// is the machine on which `mxcli new` handed MxBuild --java-home=/usr and hung. +func fakeMacJDKEnv(javaHomes map[int]string, asked *[]int) jdkEnv { + installed := map[string]int{} + for m, h := range javaHomes { + installed[h] = m + } + return jdkEnv{ + goos: "darwin", + getenv: func(string) string { return "" }, + macJavaHome: func(major int) (string, error) { + if asked != nil { + *asked = append(*asked, major) + } + if h, ok := javaHomes[major]; ok { + return h, nil + } + return "", errors.New("Unable to find any JVMs matching version \"" + strconv.Itoa(major) + "\".") + }, + lookPath: func(string) (string, error) { return "/usr/bin/java", nil }, + evalSymlinks: func(p string) (string, error) { return p, nil }, // the stub is a real file + isJDK: func(home string, major int) bool { + if home == "/usr" { + // The stub forwards to the default JDK; pretend every major + // is "default", the worst case. + return true + } + return installed[home] == major + }, + glob: func(string) ([]string, error) { return nil, nil }, + searchPaths: func(major int) []string { return jdkSearchPathsFor("darwin", major) }, + versionOf: func(string) string { return "(unknown)" }, + } +} + +func TestResolveJDK_Darwin_AsksJavaHomeForProjectMajor(t *testing.T) { + const jdk25 = "/Library/Java/JavaVirtualMachines/temurin-25.jdk/Contents/Home" + const jdk21 = "/Library/Java/JavaVirtualMachines/temurin-21.jdk/Contents/Home" + var asked []int + env := fakeMacJDKEnv(map[int]string{21: jdk21, 25: jdk25}, &asked) + + got, err := resolveJDKWith(env, 25) + if err != nil { + t.Fatalf("resolveJDK(25): %v", err) + } + if got != jdk25 { + t.Fatalf("resolveJDK(25) = %q, want java_home -v 25's answer %q", got, jdk25) + } + if len(asked) != 1 || asked[0] != 25 { + t.Fatalf("java_home was asked for %v, want [25]", asked) + } +} + +func TestResolveJDK_Darwin_NeverReturnsUsrStub(t *testing.T) { + // JDK 21 installed, project wants 25: java_home -v 25 fails, and the old + // code fell through to /usr/bin/java -> home /usr, which the stub "passed". + env := fakeMacJDKEnv(map[int]string{21: "/Library/Java/JavaVirtualMachines/temurin-21.jdk/Contents/Home"}, nil) + + got, err := resolveJDKWith(env, 25) + if got == "/usr" { + t.Fatalf("resolveJDK(25) returned the /usr/bin/java stub's home /usr") + } + if err == nil { + t.Fatalf("resolveJDK(25) = %q, want an error (no JDK 25 installed)", got) + } + msg := err.Error() + for _, want := range []string{"JDK 25 not found", "java_home -v 25", "temurin@25"} { + if !strings.Contains(msg, want) { + t.Errorf("error does not mention %q:\n%s", want, msg) + } + } +} + +func TestResolveJDK_Darwin_RejectsJavaHomeEnvAtUsr(t *testing.T) { + env := fakeMacJDKEnv(nil, nil) + env.getenv = func(k string) string { + if k == "JAVA_HOME" { + return "/usr" + } + return "" + } + if got, err := resolveJDKWith(env, 21); err == nil || got == "/usr" { + t.Fatalf("resolveJDK(21) with JAVA_HOME=/usr = (%q, %v), want an error", got, err) + } +} + +// Control: on Linux a PATH java whose home is a real JDK still resolves, and +// /usr is not special there (a distro JDK can legitimately live under it via +// symlinks resolved to /usr/lib/jvm/...). +func TestResolveJDK_Linux_PathFallbackStillWorks(t *testing.T) { + env := fakeMacJDKEnv(nil, nil) + env.goos = "linux" + env.macJavaHome = func(int) (string, error) { t.Fatal("java_home must not run off macOS"); return "", nil } + env.evalSymlinks = func(string) (string, error) { return "/usr/lib/jvm/temurin-25-jdk/bin/java", nil } + env.isJDK = func(home string, major int) bool { return home == "/usr/lib/jvm/temurin-25-jdk" && major == 25 } + got, err := resolveJDKWith(env, 25) + if err != nil || got != "/usr/lib/jvm/temurin-25-jdk" { + t.Fatalf("resolveJDK(25) on linux = (%q, %v)", got, err) + } +} diff --git a/cmd/mxcli/docker/mxbuild_watchdog.go b/cmd/mxcli/docker/mxbuild_watchdog.go new file mode 100644 index 0000000000..a45abb1faa --- /dev/null +++ b/cmd/mxcli/docker/mxbuild_watchdog.go @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "bytes" + "fmt" + "io" + "os" + "os/exec" + "strings" + "sync" + "time" +) + +// mxbuild_watchdog.go bounds a one-shot MxBuild run. Without it, `mxcli new` +// sat for 10+ minutes with Gradle spinning at 100% CPU behind a wrong +// --java-home (ChipCoV8 #6): cmd.Run() had no deadline and nothing noticed that +// the build had stopped saying anything. + +const ( + // DefaultMxBuildTimeout caps a whole build. A cold first build of a large + // project downloads Gradle dependencies, so this is generous: a false kill of + // a slow healthy build costs more than a late one of a hung build. + DefaultMxBuildTimeout = 30 * time.Minute + // DefaultMxBuildStall is how long a build may go without printing a line + // before it is presumed hung. Gradle steps can be quiet for minutes. + DefaultMxBuildStall = 10 * time.Minute + + mxbuildTimeoutEnv = "MXCLI_MXBUILD_TIMEOUT" + mxbuildStallEnv = "MXCLI_MXBUILD_STALL_TIMEOUT" +) + +// watchdogLimits are the two bounds; a zero disables that bound. +type watchdogLimits struct { + Timeout time.Duration + Stall time.Duration +} + +// mxbuildWatchdogLimits is what the one-shot MxBuild runs use; a variable so a +// test can shrink the bounds to milliseconds. +var mxbuildWatchdogLimits = mxbuildLimits + +// mxbuildLimits reads the limits from the environment (Go durations such as +// "30m"; "0" disables), falling back to the defaults. +func mxbuildLimits() watchdogLimits { + return watchdogLimits{ + Timeout: durationEnv(mxbuildTimeoutEnv, DefaultMxBuildTimeout), + Stall: durationEnv(mxbuildStallEnv, DefaultMxBuildStall), + } +} + +func durationEnv(key string, def time.Duration) time.Duration { + v := strings.TrimSpace(os.Getenv(key)) + if v == "" { + return def + } + if v == "0" { + return 0 + } + d, err := time.ParseDuration(v) + if err != nil || d < 0 { + return def + } + return d +} + +// WatchdogError reports a build the watchdog killed. +type WatchdogError struct { + Reason string // "timed out" or "stalled" + Limit time.Duration + Elapsed time.Duration + JavaHome string + Tail string // the last lines of output + EnvVar string // the variable that raises the limit +} + +func (e *WatchdogError) Error() string { + var b strings.Builder + if e.Reason == "stalled" { + fmt.Fprintf(&b, "mxbuild stalled: no output for %s (after %s), killed", e.Limit, e.Elapsed.Round(time.Second)) + } else { + fmt.Fprintf(&b, "mxbuild timed out after %s, killed", e.Limit) + } + if e.JavaHome != "" { + fmt.Fprintf(&b, "\n Java home used: %s", e.JavaHome) + } + fmt.Fprintf(&b, "\n Raise the limit with %s= (e.g. 30m; 0 disables) if the build is merely slow.", e.EnvVar) + if e.Tail != "" { + b.WriteString("\n Last output:\n") + for _, l := range strings.Split(e.Tail, "\n") { + b.WriteString(" " + l + "\n") + } + } else { + b.WriteString("\n (mxbuild printed nothing)") + } + return strings.TrimRight(b.String(), "\n") +} + +// buildActivity records when a build last wrote output, and keeps the tail of +// it, across both of its streams. +type buildActivity struct { + mu sync.Mutex + last time.Time + tail []byte +} + +const buildTailBytes = 16 << 10 + +func (a *buildActivity) saw(p []byte) { + a.mu.Lock() + defer a.mu.Unlock() + a.last = time.Now() + a.tail = append(a.tail, p...) + if len(a.tail) > buildTailBytes { + a.tail = append([]byte(nil), a.tail[len(a.tail)-buildTailBytes:]...) + } +} + +func (a *buildActivity) lastWrite() time.Time { + a.mu.Lock() + defer a.mu.Unlock() + return a.last +} + +func (a *buildActivity) tailLines(n int) string { + a.mu.Lock() + defer a.mu.Unlock() + return lastLines(string(bytes.TrimRight(a.tail, "\n")), n) +} + +// activityWriter forwards to w (which may be nil) and reports each write. +type activityWriter struct { + act *buildActivity + w io.Writer +} + +func (a activityWriter) Write(p []byte) (int, error) { + a.act.saw(p) + if a.w == nil { + return len(p), nil + } + return a.w.Write(p) +} + +// runWithWatchdog starts cmd with its stdout/stderr going to the writers it +// already has, and kills its whole process tree if it runs past lim.Timeout or +// prints nothing for lim.Stall. MxBuild is a wrapper around a JVM that runs +// Gradle, so killing only the direct child would leave the spinning +// grandchildren — and the pipe they hold would keep Wait from returning. +func runWithWatchdog(cmd *exec.Cmd, lim watchdogLimits, javaHome string) error { + act := &buildActivity{last: time.Now()} + if cmd.Stdout != nil && cmd.Stdout == cmd.Stderr { + shared := activityWriter{act, cmd.Stdout} + cmd.Stdout, cmd.Stderr = shared, shared + } else { + cmd.Stdout = activityWriter{act, cmd.Stdout} + cmd.Stderr = activityWriter{act, cmd.Stderr} + } + setProcessGroup(cmd) + // Once the tree is killed nothing should hold the pipes, but never let a + // stray descendant turn a kill into a second hang. + cmd.WaitDelay = 10 * time.Second + + start := time.Now() + if err := cmd.Start(); err != nil { + return err + } + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + tick := time.Second + if lim.Stall > 0 && lim.Stall/4 < tick { + tick = max(lim.Stall/4, 10*time.Millisecond) + } + if lim.Timeout > 0 && lim.Timeout/4 < tick { + tick = max(lim.Timeout/4, 10*time.Millisecond) + } + ticker := time.NewTicker(tick) + defer ticker.Stop() + for { + select { + case err := <-done: + return err + case now := <-ticker.C: + var werr *WatchdogError + switch { + case lim.Timeout > 0 && now.Sub(start) >= lim.Timeout: + werr = &WatchdogError{Reason: "timed out", Limit: lim.Timeout, EnvVar: mxbuildTimeoutEnv} + case lim.Stall > 0 && now.Sub(act.lastWrite()) >= lim.Stall: + werr = &WatchdogError{Reason: "stalled", Limit: lim.Stall, EnvVar: mxbuildStallEnv} + } + if werr == nil { + continue + } + _ = killProcessGroup(cmd.Process) + <-done + werr.Elapsed = time.Since(start) + werr.JavaHome = javaHome + werr.Tail = act.tailLines(20) + return werr + } + } +} diff --git a/cmd/mxcli/docker/mxbuild_watchdog_test.go b/cmd/mxcli/docker/mxbuild_watchdog_test.go new file mode 100644 index 0000000000..170ce232a2 --- /dev/null +++ b/cmd/mxcli/docker/mxbuild_watchdog_test.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows + +package docker + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// A build that prints and then goes silent — the shape of Gradle spinning +// behind a wrong --java-home — must be killed by the stall bound, with the +// last output and the java home in the error. +func TestRunWithWatchdog_StallKillsSilentBuild(t *testing.T) { + // The grandchild `sleep` stands in for the JVM under mxbuild's wrapper: it + // holds the output pipe, so only a process-tree kill lets Wait return. + cmd := exec.Command("sh", "-c", "echo starting gradle; echo daemon up; sleep 60 & wait") + out := &syncBuffer{} + cmd.Stdout, cmd.Stderr = out, out + + start := time.Now() + err := runWithWatchdog(cmd, watchdogLimits{Timeout: time.Minute, Stall: 300 * time.Millisecond}, "/usr") + elapsed := time.Since(start) + + var werr *WatchdogError + if !errors.As(err, &werr) { + t.Fatalf("err = %v, want a *WatchdogError", err) + } + if werr.Reason != "stalled" { + t.Errorf("reason = %q, want stalled", werr.Reason) + } + if elapsed > 15*time.Second { + t.Errorf("took %s to give up on a 300ms stall — the process tree was not killed", elapsed) + } + msg := err.Error() + for _, want := range []string{"daemon up", "Java home used: /usr", mxbuildStallEnv} { + if !strings.Contains(msg, want) { + t.Errorf("error does not mention %q:\n%s", want, msg) + } + } + if !strings.Contains(out.String(), "starting gradle") { + t.Errorf("output was not forwarded to the caller's writer: %q", out.String()) + } +} + +// A build that keeps printing is not stalled, but the overall bound still +// stops it. +func TestRunWithWatchdog_TimeoutKillsChattyBuild(t *testing.T) { + cmd := exec.Command("sh", "-c", "while true; do echo tick; sleep 0.05; done") + err := runWithWatchdog(cmd, watchdogLimits{Timeout: 500 * time.Millisecond, Stall: time.Minute}, "/jdk") + var werr *WatchdogError + if !errors.As(err, &werr) || werr.Reason != "timed out" { + t.Fatalf("err = %v, want a timeout", err) + } + if !strings.Contains(err.Error(), "tick") || !strings.Contains(err.Error(), mxbuildTimeoutEnv) { + t.Errorf("error lacks the tail or the env var:\n%s", err) + } +} + +// Control: a build that finishes inside both bounds returns its own result. +func TestRunWithWatchdog_PassesThroughNormalExit(t *testing.T) { + if err := runWithWatchdog(exec.Command("sh", "-c", "echo ok"), watchdogLimits{Timeout: time.Minute, Stall: time.Minute}, ""); err != nil { + t.Fatalf("clean exit: %v", err) + } + err := runWithWatchdog(exec.Command("sh", "-c", "exit 3"), watchdogLimits{Timeout: time.Minute, Stall: time.Minute}, "") + var ee *exec.ExitError + if !errors.As(err, &ee) || ee.ExitCode() != 3 { + t.Fatalf("exit 3: err = %v, want the ExitError", err) + } +} + +func TestMxBuildLimits_Env(t *testing.T) { + t.Setenv(mxbuildTimeoutEnv, "30m") + t.Setenv(mxbuildStallEnv, "0") + got := mxbuildLimits() + if got.Timeout != 30*time.Minute || got.Stall != 0 { + t.Fatalf("limits = %+v", got) + } + t.Setenv(mxbuildTimeoutEnv, "bogus") + t.Setenv(mxbuildStallEnv, "") + got = mxbuildLimits() + if got.Timeout != DefaultMxBuildTimeout || got.Stall != DefaultMxBuildStall { + t.Fatalf("defaults = %+v", got) + } +} + +// The docker build's MxBuild invocation is bounded: a fake mxbuild that prints +// one line and then hangs (with a grandchild holding the pipe) is killed. +func TestMxBuildCmd_HungBuildIsKilled(t *testing.T) { + dir := t.TempDir() + fake := filepath.Join(dir, "mxbuild") + if err := os.WriteFile(fake, []byte("#!/bin/sh\necho 'Starting Gradle'\nsleep 60 &\nwait\n"), 0o755); err != nil { + t.Fatal(err) + } + orig := mxbuildWatchdogLimits + mxbuildWatchdogLimits = func() watchdogLimits { return watchdogLimits{Timeout: time.Minute, Stall: 300 * time.Millisecond} } + t.Cleanup(func() { mxbuildWatchdogLimits = orig }) + + done := make(chan error, 1) + var out syncBuffer + go func() { done <- mxbuildCmd(fake, "/usr", dir, filepath.Join(dir, "App.mpr"), &out, &out) }() + select { + case err := <-done: + var werr *WatchdogError + if !errors.As(err, &werr) { + t.Fatalf("err = %v, want a *WatchdogError", err) + } + if !strings.Contains(err.Error(), "Starting Gradle") || !strings.Contains(err.Error(), "/usr") { + t.Errorf("error lacks the tail or the java home:\n%s", err) + } + case <-time.After(20 * time.Second): + t.Fatal("mxbuildCmd did not return: the hung build was never killed") + } +} diff --git a/cmd/mxcli/docker/portguard.go b/cmd/mxcli/docker/portguard.go index 16dd4f0c5d..17b7d20aa6 100644 --- a/cmd/mxcli/docker/portguard.go +++ b/cmd/mxcli/docker/portguard.go @@ -5,6 +5,7 @@ package docker import ( "fmt" "strings" + "time" ) // portCulpritAdvice renders the "how do I get out of this" half of a @@ -58,3 +59,19 @@ func portCulpritAdvice(port int, host string, appPort int) string { "pick another port rather than killing it.\n") return b.String() } + +// CheckPortFree reports whether something already answers on 127.0.0.1:port, +// and if so returns an error that names the port, what it was for, and the flag +// that moves it — plus the holding process when it can be resolved. +// +// It is the fail-fast form of the guard StartLocalApp applies: a caller that +// has work to do before booting (`mxcli test --local` injects into the project +// first) runs it up front, so a taken port is reported before anything is +// touched and with the flag the user can actually reach for. +func CheckPortFree(port int, role, flag string) error { + if err := pingTCP(fmt.Sprintf("127.0.0.1:%d", port), 300*time.Millisecond); err != nil { + return nil + } + return fmt.Errorf("port %d (%s) is already in use — pass %s to use another.\n%s", + port, role, flag, portCulpritAdvice(port, "127.0.0.1", port)) +} diff --git a/cmd/mxcli/docker/settle.go b/cmd/mxcli/docker/settle.go index c977f6d1af..fa104e8816 100644 --- a/cmd/mxcli/docker/settle.go +++ b/cmd/mxcli/docker/settle.go @@ -3,6 +3,7 @@ package docker import ( + "errors" "fmt" "io" "os/exec" @@ -54,7 +55,11 @@ func SettleGeneratedSources(projectPath, mxPath, version string, w io.Writer) er out := &syncBuffer{} cmd.Stdout = out cmd.Stderr = out - if err := cmd.Run(); err != nil { + if err := runWithWatchdog(cmd, mxbuildWatchdogLimits(), javaHome); err != nil { + var werr *WatchdogError + if errors.As(err, &werr) { + return err // already carries the tail and the java home + } return fmt.Errorf("mxbuild failed: %w\n%s", err, lastLines(out.String(), 20)) } fmt.Fprintln(w, " Generated sources are in their post-build form.") diff --git a/cmd/mxcli/exec_preflight.go b/cmd/mxcli/exec_preflight.go index 5068ad8225..3d25b9fb10 100644 --- a/cmd/mxcli/exec_preflight.go +++ b/cmd/mxcli/exec_preflight.go @@ -130,7 +130,11 @@ func execPreflight(exec *executor.Executor, prog *ast.Program, projectPath, scri // splice under mdl 1, an alter whose patch fails. `check -p` already // runs this verdict; exec did not, so it wrote every statement before // the refused one and none after, leaving the model half-applied. - verdicts := exec.CheckFlowVerdicts(prog) + flowReport := exec.CheckFlowVerdictsReport(prog) + verdicts := flowReport.Violations + if len(flowReport.Unpredicted) > 0 { + fmt.Fprint(w, executor.FormatUnpredictedFlows(flowReport.Unpredicted)) + } if len(verdicts) > 0 { (&linter.TextFormatter{UseColor: color}).Format(verdicts, w) if n := linter.Summarize(verdicts).Errors; n > 0 && !continueOnError { diff --git a/cmd/mxcli/exec_preflight_script_flows_test.go b/cmd/mxcli/exec_preflight_script_flows_test.go new file mode 100644 index 0000000000..2207071656 --- /dev/null +++ b/cmd/mxcli/exec_preflight_script_flows_test.go @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// A `create or modify` whose new body calls a flow the statement before it +// creates: exec's preflight said nothing, so exec wrote the new sub-flow and +// then refused the modify (MDL-V1-REBUILD, "$Reporter is still used by"), +// leaving the script half-applied. The preflight must refuse it before +// anything is written. +func TestExecPreflightRefusesModifyCallingFlowTheScriptCreates(t *testing.T) { + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + raw, err := os.ReadFile(filepath.Join(src, "PedApp.mpr")) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "PedApp.mpr"), raw, 0o644); err != nil { + t.Fatal(err) + } + if err := os.CopyFS(filepath.Join(dir, "mprcontents"), os.DirFS(filepath.Join(src, "mprcontents"))); err != nil { + t.Fatal(err) + } + mpr := filepath.Join(dir, "PedApp.mpr") + exec := executor.New(io.Discard) + exec.SetBackendFactory(func() backend.FullBackend { return modelsdkbackend.New() }) + if err := exec.Execute(&ast.ConnectStmt{Path: mpr}); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = exec.Execute(&ast.DisconnectStmt{}) }) + + parse := func(script string) *ast.Program { + t.Helper() + prog, errs := visitor.Build(script) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + return prog + } + if err := exec.ExecuteProgram(parse(`mdl 1; +create microflow MyFirstModule.VerdictAct ($F: FeedbackModule.Feedback) +begin + retrieve $Reporter from System.User first; + change $F (Subject = $Reporter/Name); + log info node 'X' 'done'; +end;`)); err != nil { + t.Fatalf("setup: %v", err) + } + run := func(script string) (string, string) { + t.Helper() + var out bytes.Buffer + refusal := execPreflight(exec, parse(script), mpr, "s.mdl", false, false, false, deprecation.Warn, &out, false) + return refusal, out.String() + } + + refusal, out := run(`mdl 1; +create microflow MyFirstModule.VerdictNewSub ($F: FeedbackModule.Feedback) +begin + retrieve $Reporter from System.User first; + change $F (Subject = $Reporter/Name); +end; +create or modify microflow MyFirstModule.VerdictAct ($F: FeedbackModule.Feedback) +begin + call microflow MyFirstModule.VerdictNewSub (F = $F); + log info node 'X' 'done'; +end;`) + if refusal == "" || !strings.Contains(refusal, "Nothing was written") { + t.Fatalf("want the pre-flight to refuse the script, got %q\n%s", refusal, out) + } + if !strings.Contains(out, "is still used by") { + t.Errorf("pre-flight output must carry exec's refusal:\n%s", out) + } + if strings.Contains(out, "not predicted") { + t.Errorf("the change is predicted, yet reported unpredicted:\n%s", out) + } + + // Control: a modify that only changes a log message is spliced; the + // pre-flight lets it run and reports nothing unpredicted. + refusal, out = run(`mdl 1; +create or modify microflow MyFirstModule.VerdictAct ($F: FeedbackModule.Feedback) +begin + retrieve $Reporter from System.User first; + change $F (Subject = $Reporter/Name); + log info node 'X' 'finished'; +end;`) + if refusal != "" || strings.Contains(out, "not predicted") { + t.Errorf("control: want the splice accepted silently, got %q\n%s", refusal, out) + } +} diff --git a/cmd/mxcli/init.go b/cmd/mxcli/init.go index 538b3247a5..d8b322d6ab 100644 --- a/cmd/mxcli/init.go +++ b/cmd/mxcli/init.go @@ -169,6 +169,7 @@ Container Runtime: // repo the odds of it being what you meant are zero. // (mxcli-formula1 findings #3.) mprFile := findMprFile(absDir) + mprPlaceholder := false if mprFile == "" { candidates := findMprFilesInSubdirs(absDir) switch len(candidates) { @@ -176,6 +177,7 @@ Container Runtime: 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" + mprPlaceholder = true case 1: absDir = filepath.Dir(candidates[0]) mprFile = filepath.Base(candidates[0]) @@ -190,7 +192,13 @@ Container Runtime: os.Exit(1) } } - projectName := filepath.Base(absDir) + // The app's name (from the .mpr), not the folder's: a cloud session + // clones into a folder named after the repository. Every use below is + // a display name; paths use absDir/mprFile directly. + projectName := projectDisplayName(absDir, mprFile) + if mprPlaceholder { + projectName = filepath.Base(absDir) + } // Determine which tools to initialize tools := initTools diff --git a/cmd/mxcli/init_appname_test.go b/cmd/mxcli/init_appname_test.go new file mode 100644 index 0000000000..0cb99ea8bd --- /dev/null +++ b/cmd/mxcli/init_appname_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The project name in generated docs is the app's, from the .mpr — not the +// repository folder's, which in a cloud session is whatever the repo was +// called (a seed repo named "seed" titled its CLAUDE.md "Mendix Project: seed"). +func TestProjectDisplayName(t *testing.T) { + cases := []struct{ dir, mpr, want string }{ + {"/work/seed", "ChipCoV8.mpr", "ChipCoV8"}, + {"/work/seed", "", "seed"}, + {"/work/seed", ".mpr", "seed"}, + {"/work/App", "App.mpr", "App"}, + } + for _, c := range cases { + if got := projectDisplayName(c.dir, c.mpr); got != c.want { + t.Errorf("projectDisplayName(%q, %q) = %q, want %q", c.dir, c.mpr, got, c.want) + } + } +} + +func TestInit_UsesAppNameNotFolderName(t *testing.T) { + dir := filepath.Join(t.TempDir(), "seed") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + mustWrite(t, filepath.Join(dir, "ChipCoV8.mpr"), "") + runInit(t, []string{"claude"}, dir) + + claude := mustRead(t, filepath.Join(dir, "CLAUDE.md")) + if !strings.Contains(claude, "# Mendix Project: ChipCoV8\n") { + t.Errorf("CLAUDE.md heading does not name the app:\n%s", claude[:min(len(claude), 400)]) + } + if strings.Contains(claude, "Mendix Project: seed") { + t.Error("CLAUDE.md is titled after the repository folder") + } + dc := mustRead(t, filepath.Join(dir, ".devcontainer", "devcontainer.json")) + if !strings.Contains(dc, `"name": "ChipCoV8"`) { + t.Errorf("devcontainer name is not the app name:\n%s", dc) + } +} + +// A later `init --sync-skills` must not retitle the doc after the folder. +func TestSyncProjectTooling_UsesAppName(t *testing.T) { + dir := filepath.Join(t.TempDir(), "seed") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + mustWrite(t, filepath.Join(dir, "ChipCoV8.mpr"), "") + path := filepath.Join(dir, "CLAUDE.md") + mustWrite(t, path, wrapMxcliSection("# Mendix Project: old\n")) + if _, err := syncProjectTooling(dir); err != nil { + t.Fatal(err) + } + if got := mustRead(t, path); !strings.Contains(got, "# Mendix Project: ChipCoV8\n") { + t.Errorf("synced CLAUDE.md heading does not name the app:\n%s", got[:min(len(got), 400)]) + } +} diff --git a/cmd/mxcli/init_claudemd.go b/cmd/mxcli/init_claudemd.go index 03af98515d..e638d6968f 100644 --- a/cmd/mxcli/init_claudemd.go +++ b/cmd/mxcli/init_claudemd.go @@ -5,6 +5,7 @@ package main import ( "fmt" + "path/filepath" "strings" ) @@ -115,6 +116,17 @@ func renderProjectGates(mprPath string) string { return sb.String() } +// projectDisplayName is the name generated docs give the project: the app's, +// taken from the .mpr file name, falling back to the folder name when there is +// no .mpr. The folder is the repository's name in a cloud session ("seed", +// "init-bootstrap"), which says nothing about the app. +func projectDisplayName(dir, mprFile string) string { + if name := strings.TrimSuffix(filepath.Base(mprFile), ".mpr"); mprFile != "" && name != "" { + return name + } + return filepath.Base(dir) +} + func generateClaudeMD(projectName, mprFile string) string { mprPath := mprFile if mprPath == "" { diff --git a/cmd/mxcli/init_hook.go b/cmd/mxcli/init_hook.go index c2b6a9126c..d11dfbe1de 100644 --- a/cmd/mxcli/init_hook.go +++ b/cmd/mxcli/init_hook.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" ) @@ -24,8 +25,8 @@ import ( // relative to the project root. const bootstrapScriptName = ".claude/bootstrap-mxcli.sh" -// bootstrapScriptTemplate is written to bootstrapScriptName. %s is the .mpr file -// name. It is POSIX sh (no bashisms) and safe to re-run: every step is a no-op +// bootstrapScriptTemplate is written to bootstrapScriptName. The first %s is +// the .mpr file name, the second the default mxcli tag (bootstrapDefaultTag). It is POSIX sh (no bashisms) and safe to re-run: every step is a no-op // once satisfied. // // It is COMMITTED on purpose — the opposite of the mxcli binary, which @@ -43,7 +44,9 @@ const bootstrapScriptTemplate = `#!/bin/sh # the fresh clone — which is exactly why this script fetches it rather than # skipping when it is absent. # -# Pin a specific mxcli with MXCLI_TAG=vX.Y.Z (default: nightly). +# Pin a specific mxcli with MXCLI_TAG=vX.Y.Z or MXCLI_TAG=nightly. The default +# is the release that generated this script, or nightly when that was not a +# release build. # # Version guard: .ai-context/mxcli-tooling.json records the mxcli that wrote # this project's tooling (CLAUDE.md, skills, lint rules). A binary older than @@ -55,7 +58,7 @@ const bootstrapScriptTemplate = `#!/bin/sh set -e MPR='%s' -TAG="${MXCLI_TAG:-nightly}" +TAG="${MXCLI_TAG:-%s}" STAMP=.ai-context/mxcli-tooling.json # stamp_field KEY: a string field of the tooling stamp, or nothing. @@ -198,9 +201,58 @@ fi # fatal, since a refresh must not block the session. ./mxcli init --sync-skills . || true -exec ./mxcli run --local --setup --ensure-db -p "$MPR" +# Cache MxBuild + the runtime and provision the database. Not exec'd: the +# hook runs this script under '|| true' so a failure never blocks the session, +# which also made a failed provisioning invisible. Its output is teed to a log +# so a failure can be named in a banner, and its status is kept in a file +# because a pipeline's status is its last command's in POSIX sh. +log=$(mktemp 2>/dev/null || echo "${TMPDIR:-/tmp}/mxcli-bootstrap.$$.log") +st="$log.status" +rm -f "$st" +{ ./mxcli run --local --setup --ensure-db -p "$MPR" 2>&1 || echo "$?" >"$st"; } | tee "$log" +status=0 +[ -f "$st" ] && status=$(cat "$st") +if [ "$status" -ne 0 ]; then + reason=$(grep -i 'error' "$log" | tail -n 1) + [ -n "$reason" ] || reason=$(grep -v '^[[:space:]]*$' "$log" | tail -n 1) + reason=$(echo "$reason" | sed 's/^[Ee][Rr][Rr][Oo][Rr]:[[:space:]]*//') + { + echo "" + echo "======================================================================" + echo "ERROR: the app database was not provisioned: ${reason:-./mxcli run exited $status}" + echo " The session started, but the app cannot run until this is fixed." + echo " Retry: ./mxcli run --local --setup --ensure-db -p $MPR" + echo " Or boot on the built-in database (no PostgreSQL needed):" + echo " ./mxcli run --local --db-type hsqldb -p $MPR" + echo "======================================================================" + } >&2 +fi +rm -f "$log" "$st" +exit "$status" ` +// releaseTagRe matches a plain release version: what a tagged release +// build carries in main.Version, and a tag that exists on GitHub. +var releaseTagRe = regexp.MustCompile(`^v[0-9]+\.[0-9]+\.[0-9]+$`) + +// bootstrapDefaultTag is the mxcli tag the bootstrap script fetches when +// MXCLI_TAG is unset: the release that generated it, so a re-fetch after an +// idle reap brings back the mxcli the project's tooling was written by rather +// than whatever nightly is current. A dev build (v0.24.0-888-g4ba1495f2), a +// nightly or an unversioned build has no fixed download, so it keeps nightly. +func bootstrapDefaultTag(version string) string { + if releaseTagRe.MatchString(version) { + return version + } + return "nightly" +} + +// renderBootstrapScript is the bootstrap script for a project's .mpr, with the +// default tag taken from this binary's version. +func renderBootstrapScript(mprFile string) string { + return fmt.Sprintf(bootstrapScriptTemplate, mprFile, bootstrapDefaultTag(Version)) +} + // sessionStartHookMarkers identify OUR hook command — so re-running init updates // it instead of duplicating it, and a user's own SessionStart hooks are never // touched. The first is the current form; the rest are older spellings kept so @@ -278,7 +330,7 @@ func writeBootstrapScript(claudeDir, mprFile string) (changed bool, err error) { // claudeDir is /.claude and bootstrapScriptName is project-relative, // so take the base name to land beside settings.json. path := filepath.Join(claudeDir, filepath.Base(bootstrapScriptName)) - want := []byte(fmt.Sprintf(bootstrapScriptTemplate, mprFile)) + want := []byte(renderBootstrapScript(mprFile)) if existing, readErr := os.ReadFile(path); readErr == nil && bytes.Equal(existing, want) { return false, nil } diff --git a/cmd/mxcli/init_hook_failure_test.go b/cmd/mxcli/init_hook_failure_test.go new file mode 100644 index 0000000000..dff8a0769d --- /dev/null +++ b/cmd/mxcli/init_hook_failure_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// fakeMxcliFailingRun is a stand-in mxcli whose `run` fails the way a session +// without a Postgres superuser does; every other command succeeds. +func fakeMxcliFailingRun(t *testing.T, path string) { + t.Helper() + body := "#!/bin/sh\n" + + "if [ \"$1\" = \"--version\" ]; then echo \"mxcli version v0.25.0 (2026-10-01T00:00:00Z)\"; exit 0; fi\n" + + "if [ \"$1\" = \"run\" ]; then echo \"Caching MxBuild...\"; echo \"Error: no local PostgreSQL superuser available to create the app database\" >&2; exit 3; fi\n" + + "exit 0\n" + if err := os.WriteFile(path, []byte(body), 0o755); err != nil { + t.Fatal(err) + } +} + +// The hook runs the script under `|| true` so a failure never blocks the +// session — which made a failed database provisioning invisible. The script +// must say so loudly itself, and still exit with the failure's status. +func TestBootstrapScript_ProvisioningFailureIsLoud(t *testing.T) { + requireSh(t) + e := newBootEnv(t, "") + fakeMxcliFailingRun(t, filepath.Join(e.project, "mxcli")) + + cmd := exec.Command("sh", bootstrapScriptName) + cmd.Dir = e.project + cmd.Env = append(os.Environ(), "PATH="+e.bin+":/usr/bin:/bin", "BOOT_LOG="+e.log, "MXCLI_TAG=") + var stdout, stderr strings.Builder + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != 3 { + t.Fatalf("script exit = %v, want status 3 from the failed run\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String()) + } + got := stderr.String() + for _, want := range []string{ + "ERROR: the app database was not provisioned", + "no local PostgreSQL superuser available", + "./mxcli run --local --setup --ensure-db -p App.mpr", + "--db-type hsqldb", + } { + if !strings.Contains(got, want) { + t.Errorf("stderr lacks %q:\n%s", want, got) + } + } + // The command's own output still reaches the session. + if !strings.Contains(stdout.String()+got, "Caching MxBuild") { + t.Errorf("run output was swallowed:\nstdout:\n%s\nstderr:\n%s", stdout.String(), got) + } +} + +// Control: a successful run prints no banner and exits 0. +func TestBootstrapScript_ProvisioningSuccessIsQuiet(t *testing.T) { + requireSh(t) + e := newBootEnv(t, "") + fakeMxcli(t, filepath.Join(e.project, "mxcli"), "v0.25.0") + out := e.run(t) + if strings.Contains(out, "ERROR") { + t.Errorf("banner on success:\n%s", out) + } + log, _ := os.ReadFile(e.log) + if !strings.Contains(string(log), "run --local --setup --ensure-db -p App.mpr") { + t.Errorf("run was not invoked:\n%s", log) + } +} diff --git a/cmd/mxcli/init_hook_test.go b/cmd/mxcli/init_hook_test.go index 087f99087e..631406445b 100644 --- a/cmd/mxcli/init_hook_test.go +++ b/cmd/mxcli/init_hook_test.go @@ -140,3 +140,51 @@ func TestEnsureSessionStartHook_InvalidJSONUntouched(t *testing.T) { t.Errorf("invalid settings.json was modified: %q", data) } } + +// The bootstrap script defaults MXCLI_TAG to the release that generated it: +// a project made by v0.25.0 must not silently pick up whatever nightly is +// current when a reaped session re-fetches the binary. Anything that is not +// a plain release has no fixed download, so it keeps nightly. +func TestBootstrapDefaultTag(t *testing.T) { + cases := map[string]string{ + "v0.25.0": "v0.25.0", + "v1.2.3": "v1.2.3", + "": "nightly", + "dev": "nightly", + "4ba1495f2": "nightly", + "v0.24.0-888-g4ba1495f2": "nightly", + "v0.24.0-dirty": "nightly", + "nightly-20261011-abcdef0": "nightly", + "0.25.0": "nightly", + "v0.25": "nightly", + } + for in, want := range cases { + if got := bootstrapDefaultTag(in); got != want { + t.Errorf("bootstrapDefaultTag(%q) = %q, want %q", in, got, want) + } + } +} + +func TestBootstrapScript_DefaultTagIsGeneratingRelease(t *testing.T) { + prev := Version + t.Cleanup(func() { Version = prev }) + + Version = "v0.25.0" + dir := t.TempDir() + if _, err := writeBootstrapScript(dir, "App.mpr"); err != nil { + t.Fatal(err) + } + script, _ := os.ReadFile(filepath.Join(dir, filepath.Base(bootstrapScriptName))) + if !strings.Contains(string(script), `TAG="${MXCLI_TAG:-v0.25.0}"`) { + t.Errorf("release build: script does not default to its own release:\n%s", script) + } + + Version = "v0.25.0-3-gabcdef0" + if _, err := writeBootstrapScript(dir, "App.mpr"); err != nil { + t.Fatal(err) + } + script, _ = os.ReadFile(filepath.Join(dir, filepath.Base(bootstrapScriptName))) + if !strings.Contains(string(script), `TAG="${MXCLI_TAG:-nightly}"`) { + t.Errorf("dev build: script does not default to nightly:\n%s", script) + } +} diff --git a/cmd/mxcli/init_hook_version_test.go b/cmd/mxcli/init_hook_version_test.go index 4e6a86a206..3139cbae38 100644 --- a/cmd/mxcli/init_hook_version_test.go +++ b/cmd/mxcli/init_hook_version_test.go @@ -30,7 +30,7 @@ func requireSh(t *testing.T) { // before the first statement that acts. func bootstrapFunctions(t *testing.T) string { t.Helper() - script := fmt.Sprintf(bootstrapScriptTemplate, "App.mpr") + script := renderBootstrapScript("App.mpr") i := strings.Index(script, "\nstale=\n") if i < 0 { t.Fatal("bootstrap script has no 'stale=' line to cut the prelude at") diff --git a/cmd/mxcli/init_tooling_sync.go b/cmd/mxcli/init_tooling_sync.go index 9f91019483..a4e1f3ade0 100644 --- a/cmd/mxcli/init_tooling_sync.go +++ b/cmd/mxcli/init_tooling_sync.go @@ -247,7 +247,7 @@ func syncProjectTooling(projectDir string) (toolingSyncResult, error) { // The docs embed the .mpr path, so without one there is nothing correct to // render; init would write a placeholder, a refresh must not. if mprFile := findMprFile(projectDir); mprFile != "" { - projectName := filepath.Base(projectDir) + projectName := projectDisplayName(projectDir, mprFile) for _, doc := range ownedDocs() { r, err := writeOwnedDoc(filepath.Join(projectDir, doc.Path), doc.Content(projectName, mprFile), false) if err != nil { diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go index 8d647f3801..78479a7f1c 100644 --- a/cmd/mxcli/main.go +++ b/cmd/mxcli/main.go @@ -365,7 +365,7 @@ func init() { diffLocalCmd.Flags().IntP("width", "w", 120, "Terminal width for side-by-side format") // Describe command flags - describeCmd.Flags().StringP("format", "f", "mdl", "Output format: mdl, json, mermaid, elk") + describeCmd.Flags().StringP("format", "f", "mdl", "Output format: mdl, json, mermaid, elk, svg") // Search command flags searchCmd.Flags().StringP("format", "f", "table", "Output format: table, names (for JSON use --json; \"json\" is accepted as a deprecated alias)") @@ -420,6 +420,7 @@ func init() { testRunCmd.Flags().StringArray("constant", nil, "With --local, set a constant for THIS RUN only: Module.Name=value (repeatable). Never written to the project. The value is visible in shell history and in `ps` — for a value that must not be, see docs/11-proposals/PROPOSAL_constant_values.md") testRunCmd.Flags().String("mxbuild-path", "", "With --local, the mxbuild to build with, overriding resolution (Studio Pro's bundled mxbuild on macOS/Windows, the cached CDN download on Linux); also settable as MXCLI_MXBUILD_PATH") testRunCmd.Flags().String("db-type", "", "With --local, the database the scratch app boots on: postgresql (default) or hsqldb (the runtime's built-in file database — no server and no provisioning, so it works without PostgreSQL). Same values as 'run --local --db-type'") + registerTestPortFlags(testRunCmd) testRunCmd.Flags().BoolP("verbose", "v", false, "Show all runtime log output") testRunCmd.Flags().BoolP("color", "", false, "Use colored output") testRunCmd.Flags().StringP("timeout", "t", "5m", "Timeout for runtime startup and test execution") diff --git a/cmd/mxcli/syntax/example_parses_test.go b/cmd/mxcli/syntax/example_parses_test.go index 4166589042..1a1de93f93 100644 --- a/cmd/mxcli/syntax/example_parses_test.go +++ b/cmd/mxcli/syntax/example_parses_test.go @@ -7,6 +7,9 @@ import ( "testing" "github.com/mendixlabs/mxcli/cmd/mxcli/syntax" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -19,7 +22,12 @@ import ( // the first place an agent looks, so a gap here becomes a wrong workaround in a // generated app rather than a puzzled user. // -// Only Example is checked. Syntax fields carry metasyntax (`GET|POST`, +// Each Example block is also held to `check --deprecations=error` in every +// context it parses in: the reference an agent copies from must not teach a +// spelling `check` warns on (MDL-DEPRnnn). Syntax fields are held to the same +// registry by TestSyntaxShowsNoDeprecatedSpelling. +// +// Only Example is parsed. Syntax fields carry metasyntax (`GET|POST`, // `[OR MODIFY]`, ``) and are not meant to parse. // // Examples come in several legitimate shapes, so each block is tried in turn as @@ -47,8 +55,14 @@ func TestExamplesParse(t *testing.T) { t.Fatalf("Example contains no MDL to check:\n%s", f.Example) } for _, block := range blocks { - if ctx, ok := parsesInSomeContext(block); ok { - t.Logf("block parses as %s", ctx) + if hosts := parsingHosts(block); len(hosts) > 0 { + t.Logf("block parses as %s", hosts[0].context) + for _, h := range hosts { + for _, v := range deprecationErrors(h.source) { + t.Errorf("example block uses a deprecated spelling (as %s): %s: %s\n--- block ---\n%s", + h.context, v.RuleID, v.Message, block) + } + } continue } _, errs := visitor.Build(block) @@ -64,6 +78,25 @@ func TestExamplesParse(t *testing.T) { } } +// deprecationErrors is what `mxcli check --deprecations=error` refuses in src: +// every MDL-DEPRnnn the validator reports, promoted to an error. Other +// violations are left alone — the probe wrappers declare nothing, so a +// reference or variable check would fail on the wrapper rather than on the +// documented spelling. +func deprecationErrors(src string) []linter.Violation { + prog, errs := visitor.Build(src) + if len(errs) > 0 || prog == nil { + return nil + } + var out []linter.Violation + for _, v := range executor.ApplyDeprecationPolicy(executor.ValidateProgram(prog, ""), deprecation.Error) { + if deprecation.IsDeprecationCode(v.RuleID) && v.Severity == linter.SeverityError { + out = append(out, v) + } + } + return out +} + // nonStatementTopics are the entries whose Example is legitimately not made of // MDL statements — the troubleshooting topics pair an error message with its fix // as prose, and `oql` documents a CLI invocation whose quoted argument is OQL, @@ -93,51 +126,93 @@ func mdlBlocks(example string) []string { return blocks } +// exampleContext is one host an Example block may be written for: the block is +// wrapped into a complete statement and parsed. +type exampleContext struct { + name string + wrap func(string) string +} + +// pageWidget hosts a widget fragment on a probe page. +func pageWidget(s string) string { + return "CREATE PAGE SyntaxDoc.Probe (Title: 'Probe', Layout: 'Atlas_Core.Atlas_Default') {\n" + s + "\n};" +} + +var exampleContexts = []exampleContext{ + {"statement", func(s string) string { return s }}, + {"microflow activity", func(s string) string { + return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + s + "\nEND;" + }}, + {"page widget", pageWidget}, + // Some page rules fire only inside their container — a data grid column's + // name, for one, is recorded as unstored (MDL-DEPR005) because the column + // sits in a grid. A fragment documented on its own is therefore also hosted + // in the containers it is written for, so those rules see it. + {"data grid column", func(s string) string { + return pageWidget("DATAGRID dgProbe (DataSource: DATABASE SyntaxDoc.Entity) {\n" + s + "\n}") + }}, + {"data view widget", func(s string) string { + return pageWidget("DATAVIEW dvProbe (DataSource: DATABASE SyntaxDoc.Entity) {\n" + s + "\n}") + }}, + {"workflow activity", func(s string) string { + return "CREATE WORKFLOW SyntaxDoc.Probe PARAMETER $Ctx: SyntaxDoc.Ctx\nBEGIN\n" + s + "\nEND WORKFLOW;" + }}, + // XPath topics illustrate a constraint on its own (`WHERE [...]`), which + // is a clause rather than a statement. Hanging it off a RETRIEVE checks + // the constraint itself without forcing the docs to repeat a full + // statement around every example. + {"retrieve clause", func(s string) string { + return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + + "RETRIEVE $Probe FROM SyntaxDoc.Entity\n" + s + ";\nEND;" + }}, +} + // parsesInSomeContext reports the first context the example parses in. func parsesInSomeContext(example string) (string, bool) { - for _, c := range []struct { - name string - wrap func(string) string - }{ - {"statement", func(s string) string { return s }}, - {"microflow activity", func(s string) string { - return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + s + "\nEND;" - }}, - {"page widget", func(s string) string { - return "CREATE PAGE SyntaxDoc.Probe (Title: 'Probe', Layout: 'Atlas_Core.Atlas_Default') {\n" + s + "\n};" - }}, - {"workflow activity", func(s string) string { - return "CREATE WORKFLOW SyntaxDoc.Probe PARAMETER $Ctx: SyntaxDoc.Ctx\nBEGIN\n" + s + "\nEND WORKFLOW;" - }}, - // XPath topics illustrate a constraint on its own (`WHERE [...]`), which - // is a clause rather than a statement. Hanging it off a RETRIEVE checks - // the constraint itself without forcing the docs to repeat a full - // statement around every example. - {"retrieve clause", func(s string) string { - return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + - "RETRIEVE $Probe FROM SyntaxDoc.Entity\n" + s + ";\nEND;" - }}, - } { - if _, errs := visitor.Build(c.wrap(example)); len(errs) == 0 { - return c.name, true + hosts := parsingHosts(example) + if len(hosts) == 0 { + return "", false + } + return hosts[0].context, true +} + +// parsedHost is an example block wrapped in a context it parses in. +type parsedHost struct { + context string + source string +} + +// parsingHosts returns every context the example parses in, with the wrapped +// source. Several can apply — a widget parses at page level and inside a data +// view — and each is a place a reader may paste it, so each is validated. +func parsingHosts(example string) []parsedHost { + var out []parsedHost + for _, c := range exampleContexts { + src := c.wrap(example) + if _, errs := visitor.Build(src); len(errs) == 0 { + out = append(out, parsedHost{c.name, src}) } } + if len(out) > 0 { + return out + } // A block may list several alternative clauses one per line (the xpath // function reference does this). Accept it when every line is a valid clause // on its own — the block is a menu, not one statement. if lines := nonEmptyLines(example); len(lines) > 1 { - all := true + var perLine []parsedHost for _, line := range lines { - if _, ok := parsesInSomeContext(line); !ok { - all = false - break + hosts := parsingHosts(line) + if len(hosts) == 0 { + return nil + } + for _, h := range hosts { + perLine = append(perLine, parsedHost{"per-line clauses: " + h.context, h.source}) } } - if all { - return "per-line clauses", true - } + return perLine } - return "", false + return nil } // stripTestAnnotations removes `/** … @test … */` blocks. diff --git a/cmd/mxcli/syntax/features_domain_model.go b/cmd/mxcli/syntax/features_domain_model.go index c0f96dddf3..9dd16345bb 100644 --- a/cmd/mxcli/syntax/features_domain_model.go +++ b/cmd/mxcli/syntax/features_domain_model.go @@ -82,7 +82,7 @@ func init() { "non-persistent", "extends", "generalization", "index", "event handler", "before commit", "after commit", }, - Syntax: "CREATE PERSISTENT ENTITY Module.Name (\n Attr: Type [NOT NULL [ERROR 'msg']] [UNIQUE [ERROR 'msg']] [DEFAULT val],\n ...\n)\n[INDEX (attr1, attr2)]\n[ON BEFORE|AFTER CREATE|COMMIT|DELETE|ROLLBACK CALL Module.MF [RAISE ERROR]];\n\n-- Documentation: the /** … */ doc comment before the statement, or\n-- ALTER ENTITY Module.Name SET DOCUMENTATION 'text' on an existing entity.\n\nCREATE NON-PERSISTENT ENTITY Module.Name (...);\nCREATE PERSISTENT ENTITY Module.Name EXTENDS Module.Parent (...);\n\n-- INDEX goes AFTER the closing parenthesis, never inside the attribute list.\n-- On an existing entity, either spelling works:\nALTER ENTITY Module.Name ADD INDEX [IF NOT EXISTS] [name] [ON] (attr1 [ASC|DESC], ...);\nCREATE INDEX IdxName ON Module.Name (attr1 [ASC|DESC], ...);\n\n-- Re-runnable script: IF NOT EXISTS skips instead of erroring, and leaves an\n-- existing element untouched (unlike OR MODIFY, which rebuilds it from the\n-- statement and drops any attribute the statement omits).\nCREATE ENTITY IF NOT EXISTS Module.Name (...);\nALTER ENTITY Module.Name ADD ATTRIBUTE IF NOT EXISTS Attr: Type;\nALTER ENTITY Module.Name DROP INDEX IF EXISTS (attr1, ...);", + Syntax: "CREATE PERSISTENT ENTITY Module.Name (\n Attr: Type [NOT NULL [ERROR MESSAGE 'msg']] [UNIQUE [ERROR MESSAGE 'msg']] [DEFAULT val],\n ...\n)\n[INDEX (attr1, attr2)]\n[ON BEFORE|AFTER CREATE|COMMIT|DELETE|ROLLBACK CALL Module.MF [RAISE ERROR]];\n\n-- An AutoNumber attribute needs its start value: `Nr: AutoNumber DEFAULT 1`.\n-- Without the DEFAULT seed, check reports MDL023 and the build fails CE7247.\n\n-- Documentation: the /** … */ doc comment before the statement, or\n-- ALTER ENTITY Module.Name SET DOCUMENTATION 'text' on an existing entity.\n\nCREATE NON-PERSISTENT ENTITY Module.Name (...);\nCREATE PERSISTENT ENTITY Module.Name EXTENDS Module.Parent (...);\n\n-- INDEX goes AFTER the closing parenthesis, never inside the attribute list.\n-- On an existing entity, either spelling works:\nALTER ENTITY Module.Name ADD INDEX [IF NOT EXISTS] [name] [ON] (attr1 [ASC|DESC], ...);\nCREATE INDEX IdxName ON Module.Name (attr1 [ASC|DESC], ...);\n\n-- Re-runnable script: IF NOT EXISTS skips instead of erroring, and leaves an\n-- existing element untouched (unlike OR MODIFY, which rebuilds it from the\n-- statement and drops any attribute the statement omits).\nCREATE ENTITY IF NOT EXISTS Module.Name (...);\nALTER ENTITY Module.Name ADD ATTRIBUTE IF NOT EXISTS Attr: Type;\nALTER ENTITY Module.Name DROP INDEX IF EXISTS (attr1, ...);", Example: "mdl 1;\n-- Persistent with constraints and index\nCREATE PERSISTENT ENTITY Shop.Order (\n OrderNumber: String(20) NOT NULL,\n Total: Decimal DEFAULT 0,\n CreatedAt: DateTime\n)\nINDEX (OrderNumber)\nON BEFORE COMMIT CALL Shop.ValidateOrder($currentObject) RAISE ERROR;\n\n-- With generalization\nCREATE PERSISTENT ENTITY Shop.ProductImage EXTENDS System.Image (\n Caption: String(200)\n);", SeeAlso: []string{"domain-model.entity.alter", "domain-model.entity.attributes"}, }) @@ -124,7 +124,7 @@ func init() { "localized", "not localized", "localize date", "auto owner", "auto changed by", "system attribute", }, - Syntax: "-- Data types\nString(n) Integer Long Decimal Boolean DateTime Date\nAutoNumber Binary HashedString Enumeration(Module.Name)\n\n-- System attributes (auditing)\nAutoOwner AutoChangedBy AutoCreatedDate AutoChangedDate\n\n-- Constraints\nNOT NULL [ERROR 'msg'] UNIQUE [ERROR 'msg'] DEFAULT value\nCALCULATED BY Module.Microflow\nNOT LOCALIZED | LOCALIZED DateTime only: Studio Pro's \"Localize\" (LocalizeDate).\n Unstated = localized on a new attribute; a rewrite\n (create or modify, modify attribute) keeps the stored value.\n describe prints `not localized` only.", + Syntax: "-- Data types\nString(n) Integer Long Decimal Boolean DateTime\nAutoNumber Binary HashedString Enumeration(Module.Name)\n\n-- System attributes (auditing)\nAutoOwner AutoChangedBy AutoCreatedDate AutoChangedDate\n\n-- Constraints\nNOT NULL [ERROR MESSAGE 'msg'] UNIQUE [ERROR MESSAGE 'msg'] DEFAULT value\nAutoNumber needs a DEFAULT seed (its start value): `Nr: AutoNumber DEFAULT 1` --\n without one, check reports MDL023 and the build fails CE7247\nCALCULATED BY Module.Microflow\nNOT LOCALIZED | LOCALIZED DateTime only: Studio Pro's \"Localize\" (LocalizeDate).\n Unstated = localized on a new attribute; a rewrite\n (create or modify, modify attribute) keeps the stored value.\n describe prints `not localized` only.", Example: "CREATE PERSISTENT ENTITY MyModule.AuditedEntity (\n Name: String(100) NOT NULL,\n Age: Integer DEFAULT 0,\n Price: Decimal,\n IsActive: Boolean DEFAULT true,\n Status: Enumeration(MyModule.Status),\n BirthDate: DateTime NOT LOCALIZED,\n FullName: String(200) CALCULATED BY MyModule.CalcFullName,\n Owner: AutoOwner,\n ChangedBy: AutoChangedBy,\n CreatedDate: AutoCreatedDate,\n ChangedDate: AutoChangedDate\n);", SeeAlso: []string{"domain-model.entity.create", "domain-model.types"}, }) diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index 13c236c28a..e103d8fa76 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -10,7 +10,7 @@ func init() { "microflow", "nanoflow", "logic", "automation", "action", "activity", "flow", }, - Syntax: "CREATE [OR REPLACE | OR MODIFY] MICROFLOW Module.Name ($Param: Type) RETURNS Type AS $Result\nBEGIN\n \nEND;", + Syntax: "CREATE [OR MODIFY] MICROFLOW Module.Name ($Param: Type) RETURNS Type AS $Result\nBEGIN\n \nEND;", Example: "mdl 1;\nCREATE MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n COMMIT $NewOrder;\n RETURN $NewOrder;\nEND;\n\n-- Re-runnable: replaces the microflow if it already exists\nCREATE OR MODIFY MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n RETURN $NewOrder;\nEND;", SeeAlso: []string{"microflow.create", "microflow.variables", "microflow.control-flow", "create-modifiers", "rename"}, }) @@ -616,7 +616,67 @@ func init() { }, Syntax: "$Result = CALL MICROFLOW Module.Name (Param = value) [IN QUEUE Module.Queue];\n$Result = CALL JAVA ACTION Module.Name (Param = value) [IN QUEUE Module.Queue];", Example: "$IsValid = CALL MICROFLOW MyModule.ValidateOrder (\n Order = $NewOrder\n);\n\n$Token = CALL JAVA ACTION MyModule.GenerateToken (\n UserId = $User/Id\n);\n\n-- Run the call on a task queue (background execution). The queue must\n-- exist; a queued CALL JAVA ACTION must return Nothing, or the build fails\n-- with CE7038.\nCALL MICROFLOW MyModule.ACT_Refresh () IN QUEUE MyModule.RefreshQueue;\nCALL JAVA ACTION MyModule.RefreshData (Url = $Url) IN QUEUE MyModule.RefreshQueue;\n\n-- A blank argument: the mapping is kept, its expression left empty, as\n-- Studio Pro stores an argument field left blank. Not the same as `empty`\n-- (a Mendix value); only CALL MICROFLOW and CALL NANOFLOW take it. Mendix\n-- reports it as CE0127 \"Missing argument\": it exists so DESCRIBE round-trips.\nCALL MICROFLOW MyModule.ACT_Log (Message = 'x', TraceId = nothing);", - SeeAlso: []string{"java-action", "microflow.create", "queue"}, + SeeAlso: []string{"java-action", "microflow.create", "queue", "microflow.workflow-actions"}, + }) + + Register(SyntaxFeature{ + Path: "microflow.workflow-actions", + Summary: "Start and manage workflows from a microflow: call workflow, get workflow data, workflow operation, lock/unlock", + Keywords: []string{ + "call workflow", "start workflow", "trigger workflow", "workflow from microflow", + "get workflow data", "get workflows for", "get workflow activity records", + "workflow operation", "abort workflow", "pause workflow", "unpause workflow", + "retry workflow", "restart workflow", "continue workflow", + "open workflow", "open user task", "lock workflow", "unlock workflow", + "pause all", "unpause all", "workflow actions", + }, + Syntax: "-- Microflow statements (not nanoflow ones). Each takes an optional ON ERROR clause.\n" + + "$Wf = CALL WORKFLOW Module.Workflow ($ContextObj); -- start an instance\n" + + "$Data = GET WORKFLOW DATA $Wf AS Module.Workflow; -- the instance's context object\n" + + "$Wfs = GET WORKFLOWS FOR $ContextObj; -- instances running on an object\n" + + "$Records = GET WORKFLOW ACTIVITY RECORDS $Wf; -- the instance's activity history\n" + + "WORKFLOW OPERATION ABORT $Wf [REASON ];\n" + + "WORKFLOW OPERATION PAUSE | UNPAUSE | RETRY | RESTART | CONTINUE $Wf;\n" + + "OPEN WORKFLOW $Wf; -- show the instance's admin page\n" + + "OPEN USER TASK $Task; -- show the task's page\n" + + "LOCK WORKFLOW $WfDef | Module.Workflow [PAUSE ALL];\n" + + "UNLOCK WORKFLOW $WfDef | Module.Workflow [UNPAUSE ALL];\n\n" + + "-- CALL WORKFLOW: a workflow has exactly one context parameter, so the argument\n" + + "-- is positional — the context object, not a `Param = value` list. That is the\n" + + "-- form DESCRIBE emits; the variable is a System.Workflow.\n" + + "-- $Wf is a System.Workflow, $Task a System.WorkflowUserTask, $WfDef a\n" + + "-- System.WorkflowDefinition. A lock always names its workflow: a bare\n" + + "-- `LOCK WORKFLOW ALL` is refused (MDL-WF17) — it built as CE1825. PAUSE ALL /\n" + + "-- UNPAUSE ALL are Studio Pro's \"Pause / Unpause instances\".\n" + + "-- Completing a user task and notifying a running workflow are their own\n" + + "-- topics: workflow.set-task-outcome and workflow.notify.", + Example: "mdl 1;\n" + + "create microflow HR.ACT_StartApproval ($Request: HR.Request)\n" + + "returns System.Workflow as $Workflow\n" + + "begin\n" + + " $Workflow = call workflow HR.WF_Approval ($Request);\n" + + " return $Workflow;\n" + + "end;\n\n" + + "create microflow HR.ACT_Inspect ($Request: HR.Request)\n" + + "begin\n" + + " $Workflows = get workflows for $Request;\n" + + " $Workflow = head $Workflows;\n" + + " $Data = get workflow data $Workflow as HR.WF_Approval;\n" + + " $Records = get workflow activity records $Workflow;\n" + + " open workflow $Workflow;\n" + + "end;\n\n" + + "create microflow HR.ACT_Withdraw ($Workflow: System.Workflow, $Task: System.WorkflowUserTask)\n" + + "begin\n" + + " workflow operation abort $Workflow reason 'Request withdrawn';\n" + + " open user task $Task;\n" + + "end;\n\n" + + "-- Hold every instance of a definition during a migration, then release them\n" + + "create microflow HR.ACT_Maintenance ()\n" + + "begin\n" + + " lock workflow HR.WF_Approval pause all;\n" + + " unlock workflow HR.WF_Approval unpause all;\n" + + "end;", + SeeAlso: []string{"microflow.call", "workflow.call-workflow", "workflow.set-task-outcome", "workflow.notify", "workflow"}, }) Register(SyntaxFeature{ @@ -626,7 +686,7 @@ func init() { "nanoflow", "create nanoflow", "client-side", "runs in the browser", "offline", "client logic", "disallowed in nanoflow", "nanoflow restrictions", }, - Syntax: "CREATE [OR REPLACE] NANOFLOW Module.Name ($Param: Type)\n" + + Syntax: "CREATE [OR MODIFY] NANOFLOW Module.Name ($Param: Type)\n" + "RETURNS Type AS $Result\nBEGIN\n \nEND;\n\n" + "The body is microflow syntax — every topic under `microflow` applies —\n" + "MINUS what cannot run in the browser. `mxcli check` refuses each of these\n" + diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 3853391552..302e255647 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -84,14 +84,15 @@ func init() { "or replace", "or modify", "create or replace", "create or modify", "idempotent", "upsert", "re-run", "already exists", "overwrite", }, - Syntax: "CREATE [OR REPLACE | OR MODIFY] ;\n\n" + + Syntax: "CREATE [OR MODIFY] ;\n\n" + "-- Applies to every CREATE statement — entity, microflow, page, workflow,\n" + "-- REST client, security role, and the rest. Without a modifier, creating\n" + "-- something that already exists is an error.\n" + - "-- OR REPLACE discard the existing document and write a fresh one\n" + - "-- OR MODIFY update the existing document in place\n" + - "-- Both reuse the existing element's ID, so references from other\n" + - "-- documents survive.\n" + + "-- OR MODIFY rewrite the existing document from the statement, reusing\n" + + "-- its ID, so references from other documents survive.\n" + + "-- OR REPLACE is a deprecated alias of OR MODIFY — same meaning (MDL-DEPR001;\n" + + "-- `mxcli fmt --upgrade` rewrites it). The one exception is\n" + + "-- CREATE OR REPLACE TRANSLATIONS, which replaces the whole set (see translations).\n" + "--\n" + "-- A rewrite rebuilds the document from the statement, so properties MDL\n" + "-- cannot express are CARRIED OVER rather than reset — a microflow's URL\n" + @@ -348,7 +349,7 @@ DISCONNECT;`, "offline sync", "offline entity", "pwa", "download mode", "throw error", "sync error", "partial sync", "server rejects", }, - Syntax: `CREATE OR REPLACE NAVIGATION + Syntax: `CREATE OR MODIFY NAVIGATION HOME PAGE Module.Page | HOME MICROFLOW Module.Flow | HOME NANOFLOW Module.Flow [HOME PAGE Module.Page FOR UserRole] [LOGIN PAGE Module.LoginPage] @@ -469,13 +470,13 @@ CREATE OR MODIFY NAVIGATION TabletOffline Register(SyntaxFeature{ Path: "navigation.alter", - Summary: "Modify navigation via round-trip: DESCRIBE, edit, CREATE OR REPLACE", + Summary: "Modify navigation via round-trip: DESCRIBE, edit, CREATE OR MODIFY", Keywords: []string{ "alter navigation", "modify navigation", "update navigation", "round-trip", "edit menu", }, - Syntax: "-- Round-trip workflow:\n-- 1. DESCRIBE NAVIGATION ;\n-- 2. Copy output, modify\n-- 3. Paste as CREATE OR REPLACE NAVIGATION ...", - Example: "-- Inspect current state\nDESCRIBE NAVIGATION Responsive;\n-- Copy output, modify, paste back as CREATE OR REPLACE", + Syntax: "-- Round-trip workflow:\n-- 1. DESCRIBE NAVIGATION ;\n-- 2. Copy output, modify\n-- 3. Paste as CREATE OR MODIFY NAVIGATION ...", + Example: "-- Inspect current state\nDESCRIBE NAVIGATION Responsive;\n-- Copy output, modify, paste back as CREATE OR MODIFY", SeeAlso: []string{"navigation.create", "navigation.show"}, }) @@ -687,7 +688,7 @@ LIST WORKFLOW GROUPS; -- Db2, Hsqldb, MySql, Oracle, PostgreSql, SapHana, SqlServer -- (matched case-insensitively and stored in the spelling above). --- MODEL accepts these keys (an unknown one is refused and lists them): +-- RUNTIME accepts these keys (an unknown one is refused and lists them): -- AfterStartupMicroflow, BeforeShutdownMicroflow, HealthCheckMicroflow, -- HashAlgorithm, BcryptCost, JavaVersion, RoundingMode, -- AllowUserMultipleSessions, ScheduledEventTimeZoneCode, DefaultTimeZoneCode, @@ -1012,14 +1013,15 @@ DESCRIBE STRUCTURE DEPTH 1 ALL;`, Syntax: `MOVE Module.Name TO FOLDER 'Path'; -- doctype: every top-level document, spelled as DESCRIBE spells it — -- PAGE | SNIPPET | BUILDING BLOCK | LAYOUT | MENU --- MICROFLOW | NANOFLOW | WORKFLOW | QUEUE | SCHEDULED EVENT +-- MICROFLOW | NANOFLOW | WORKFLOW | TASK QUEUE | SCHEDULED EVENT -- ENUMERATION | CONSTANT | REGULAR EXPRESSION -- JSON STRUCTURE | IMPORT MAPPING | EXPORT MAPPING -- JAVA ACTION | JAVASCRIPT ACTION | DATABASE CONNECTION | DATA TRANSFORMER -- IMAGE COLLECTION | ICON COLLECTION --- REST CLIENT | PUBLISHED REST SERVICE | ODATA CLIENT | ODATA SERVICE +-- CONSUMED REST SERVICE | PUBLISHED REST SERVICE +-- CONSUMED ODATA SERVICE | PUBLISHED ODATA SERVICE -- BUSINESS EVENT SERVICE --- MODEL | AGENT | KNOWLEDGE BASE | CONSUMED MCP SERVICE +-- AI MODEL | AGENT | KNOWLEDGE BASE | CONSUMED MCP SERVICE -- plus ENTITY (moves between domain models) and FOLDER (moves a folder). MOVE Module.Name TO TargetModule; MOVE OldModule.Name TO FOLDER 'Path' IN NewModule; diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 975effdc1c..99cb71dd23 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -128,9 +128,9 @@ LIST IMPACT OF htmlelement; "statictext", "staticimage", "dynamicimage", "dropdown", "referenceselector", "legacydatagrid", "visible", "editable", "conditional visibility", }, - Syntax: "-- Containers\nLAYOUTGRID name { ROW r { COLUMN c (DesktopWidth: 6) { ... } } }\nCONTAINER name (Class: 'cls') { ... }\nCONTAINER name (OnClick: CALL MICROFLOW Module.MF) { ... } -- clickable container\nCUSTOMCONTAINER name (Class: 'cls') { ... }\nGROUPBOX name (Caption: 'C') { ... }\nTABCONTAINER name { TABPAGE tp (Caption: 'One') { ... } TABPAGE tp2 (Caption: 'Two') { ... } }\n\n" + - "-- Data widgets\nDATAVIEW name (DataSource: $Param) { ... FOOTER { ... } }\nDATAGRID name (DataSource: DATABASE Module.Entity) { COLUMN c (Attribute: A) }\nGALLERY name (DataSource: DATABASE Module.Entity, DesktopColumns: 3) { ... }\nLISTVIEW name (DataSource: DATABASE Module.Entity) { ... }\nLISTVIEW name (...) { ... TEMPLATE FOR Module.Specialization { ... } }\n\n" + - "-- Data grid 2 column filters go INSIDE the column's own braces\nDATAGRID dg (...) { COLUMN c (Attribute: A) { TEXTFILTER tf (Attribute: A) } }\nTEXTFILTER | NUMBERFILTER | DATEFILTER | DROPDOWNFILTER | DROPDOWNSORT\n" + + Syntax: "-- Containers\nLAYOUTGRID name { ROW { COLUMN (DesktopWidth: 6) { ... } } }\nCONTAINER name (Class: 'cls') { ... }\nCONTAINER name (OnClick: CALL MICROFLOW Module.MF) { ... } -- clickable container\nCUSTOMCONTAINER name (Class: 'cls') { ... }\nGROUPBOX name (Caption: 'C') { ... }\nTABCONTAINER name { TABPAGE tp (Caption: 'One') { ... } TABPAGE tp2 (Caption: 'Two') { ... } }\n\n" + + "-- Data widgets\nDATAVIEW name (DataSource: $Param) { ... FOOTER { ... } }\nDATAGRID name (DataSource: DATABASE Module.Entity) { COLUMN (Attribute: A) }\nGALLERY name (DataSource: DATABASE Module.Entity, DesktopColumns: 3) { ... }\nLISTVIEW name (DataSource: DATABASE Module.Entity) { ... }\nLISTVIEW name (...) { ... TEMPLATE FOR Module.Specialization { ... } }\n\n" + + "-- Data grid 2 column filters go INSIDE the column's own braces\nDATAGRID dg (...) { COLUMN (Attribute: A) { TEXTFILTER tf (Attribute: A) } }\nTEXTFILTER | NUMBERFILTER | DATEFILTER | DROPDOWNFILTER | DROPDOWNSORT\n" + "-- Match the filter to the column's type, or MxBuild refuses it: String ->\n" + "-- TEXTFILTER, Integer/Long/Decimal -> NUMBERFILTER, Date and time -> DATEFILTER,\n" + "-- Enumeration AND Boolean -> DROPDOWNFILTER (the drop-down filter's own\n" + @@ -138,7 +138,7 @@ LIST IMPACT OF htmlelement; "-- A column may carry BOTH a custom-content widget and a filter — `content`\n" + "-- and `filter` are separate slots, so a read-only CHECKBOX cell and a\n" + "-- DROPDOWNFILTER live in the same braces:\n" + - "DATAGRID dg (...) { COLUMN Active (Attribute: IsActive) {\n" + + "DATAGRID dg (...) { COLUMN (Attribute: IsActive) {\n" + " CHECKBOX cb (Attribute: IsActive, Editable: Never, ReadOnlyStyle: Control)\n" + " DROPDOWNFILTER ddf } }\n" + "-- ReadOnlyStyle (Inherit | Control | Text) is what makes a read-only check\n" + @@ -339,7 +339,7 @@ LIST IMPACT OF htmlelement; "the names.\n\n" + "A bare `Class:` REPLACES the widget's classes rather than adding to them.\n" + "Read the current value first if you meant to append.\n\n" + - "Reach for ALTER STYLING rather than CREATE OR REPLACE PAGE whenever only\n" + + "Reach for ALTER STYLING rather than CREATE OR MODIFY PAGE whenever only\n" + "the look changes: replacing the page rewrites every widget in it, so the\n" + "diff is the whole document and anything MDL cannot yet spell is lost.\n\n" + "`SET Class = 'x', 'P' = ON` (no parentheses, `=`) still runs and warns\n" + @@ -408,8 +408,8 @@ LIST IMPACT OF htmlelement; "column width", "alignment", "wrap text", "visible", "dynamic cell class", "tooltip", "associated attribute", "association column", }, - Syntax: "COLUMN name (\n Attribute: AttrName, -- own attribute\n -- or an attribute over an association (bare association name):\n -- Attribute: Assoc/Attr e.g. Order_Customer/Name\n Caption: 'Header'\n [, Sortable: true|false]\n [, Resizable: true|false]\n [, Draggable: true|false]\n [, Hidable: yes|hidden|no]\n [, ColumnWidth: autoFill|autoFit|manual]\n [, Size: integer]\n [, Alignment: left|center|right]\n [, WrapText: true|false]\n [, Visible: ] -- once for the grid: a page variable or parameter, never $currentObject (MDL-WIDGET43)\n [, DynamicCellClass: ] -- per row: $currentObject is the row\n [, Tooltip: 'text']\n)", - Example: "COLUMN colPrice (\n Attribute: Price, Caption: 'Price',\n Alignment: right, Sortable: false,\n ColumnWidth: manual, Size: 150,\n Tooltip: 'Price in USD'\n)\n\n-- Associated attribute (attribute over a reference association):\nCOLUMN colCustomer (Attribute: Order_Customer/Name, Caption: 'Customer')", + Syntax: "COLUMN ( -- no name: Mendix stores none for a data grid column (MDL-DEPR005)\n Attribute: AttrName, -- own attribute\n -- or an attribute over an association (bare association name):\n -- Attribute: Assoc/Attr e.g. Order_Customer/Name\n Caption: 'Header'\n [, Sortable: true|false]\n [, Resizable: true|false]\n [, Draggable: true|false]\n [, Hidable: yes|hidden|no]\n [, ColumnWidth: autoFill|autoFit|manual]\n [, Size: integer]\n [, Alignment: left|center|right]\n [, WrapText: true|false]\n [, Visible: ] -- once for the grid: a page variable or parameter, never $currentObject (MDL-WIDGET43)\n [, DynamicCellClass: ] -- per row: $currentObject is the row\n [, Tooltip: 'text']\n)", + Example: "COLUMN (\n Attribute: Price, Caption: 'Price',\n Alignment: right, Sortable: false,\n ColumnWidth: manual, Size: 150,\n Tooltip: 'Price in USD'\n)\n\n-- Associated attribute (attribute over a reference association):\nCOLUMN (Attribute: Order_Customer/Name, Caption: 'Customer')", SeeAlso: []string{"page.widgets"}, }) @@ -477,7 +477,7 @@ LIST IMPACT OF htmlelement; "placeholder", "navigationtree", "topbar", "sidebar", "frame", "menubar", "simplemenubar", "bottom bar", "phone layout", "menu document", }, - Syntax: "CREATE [OR REPLACE] LAYOUT Module.Name (\n" + + Syntax: "CREATE [OR MODIFY] LAYOUT Module.Name (\n" + " layouttype: 'Responsive' | 'Phone' | 'Tablet' | 'ModalPopup' -- web\n" + " -- | 'Default' | 'Popup' -- native\n" + " [, class: 'layout-atlas layout-atlas-responsive-topbar']\n" + @@ -577,7 +577,7 @@ LIST IMPACT OF htmlelement; "DROP LAYOUT MyModule.App_Old;\n\n" + "-- Or correct a layout in place by re-creating it under the same name:\n" + "-- the pages stay bound by qualified name and rebind to the new document.\n" + - "-- (CREATE OR REPLACE LAYOUT does this in one statement.)", + "-- (CREATE OR MODIFY LAYOUT does this in one statement.)", SeeAlso: []string{"layout", "layout.alter", "layout.show"}, }) @@ -615,7 +615,7 @@ LIST IMPACT OF htmlelement; "-- addressed as .. Only INSERT INTO takes a region;\n" + "-- BEFORE/AFTER position a widget among siblings, so name the widget.\n" + "-- Pages in Marketplace modules are skipped and named; ALTER LAYOUT refuses\n" + - "-- a Marketplace target outright. Prefer ALTER over CREATE OR REPLACE for a\n" + + "-- a Marketplace target outright. Prefer ALTER over CREATE OR MODIFY for a\n" + "-- layout you did not author: a rewrite is only as complete as its describe.", SeeAlso: []string{"layout", "layout.show", "page.alter"}, }) diff --git a/cmd/mxcli/syntax/features_workflow.go b/cmd/mxcli/syntax/features_workflow.go index 87f6f78a4b..2abcf253f9 100644 --- a/cmd/mxcli/syntax/features_workflow.go +++ b/cmd/mxcli/syntax/features_workflow.go @@ -313,9 +313,9 @@ func init() { Keywords: []string{ "call workflow", "sub-workflow", "nested workflow", }, - Syntax: "CALL WORKFLOW Module.WF[( = , ...)] [AS ] [CAPTION ''];", + Syntax: "CALL WORKFLOW Module.WF[( = , ...)] [AS ] [CAPTION ''];\n\n-- This is the workflow ACTIVITY. To start a workflow from a microflow\n-- ($Wf = CALL WORKFLOW Module.WF ($Obj);) see microflow.workflow-actions.", Example: "CALL WORKFLOW HR.SubApproval CAPTION 'Delegate to sub-process';\n\nCALL WORKFLOW HR.SubApproval(Request = $WorkflowContext) AS callWf1;", - SeeAlso: []string{"workflow.create", "workflow.call-microflow"}, + SeeAlso: []string{"workflow.create", "workflow.call-microflow", "microflow.workflow-actions"}, }) Register(SyntaxFeature{ diff --git a/cmd/mxcli/syntax/syntax_deprecations_test.go b/cmd/mxcli/syntax/syntax_deprecations_test.go new file mode 100644 index 0000000000..908aa6873c --- /dev/null +++ b/cmd/mxcli/syntax/syntax_deprecations_test.go @@ -0,0 +1,315 @@ +// SPDX-License-Identifier: Apache-2.0 + +package syntax_test + +import ( + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/syntax" + "github.com/mendixlabs/mxcli/mdl/deprecation" +) + +// TestSyntaxShowsNoDeprecatedSpelling holds the Syntax field — metasyntax, which +// does not parse — to the deprecation registry. +// +// TestExamplesParse runs every Example through the validator, but the Syntax +// text is the part an agent reads as the grammar, and it is where the spellings +// `check` warns on survived: `NOT NULL [ERROR 'msg']` (MDL-DEPR021) and a named +// data grid `COLUMN name (` (MDL-DEPR005). +// +// The patterns come from the registry, never from a hand list, so a new entry is +// covered the day it lands. Each entry's Example and CanonicalExample differ +// only where the spelling changed; the tokens of that difference, with one token +// of context, are the deprecated spelling's fingerprint. Identifiers and string +// literals become wildcards (a Syntax text writes `name`, ``, `'msg'` +// where the example has a real one); lower-case words stay literal keywords. +// +// To keep false positives out: `--` comments are not matched (prose may name the +// old form in order to warn against it), a fingerprint must hold a keyword and +// at least two tokens, and an entry whose two examples differ in a span too wide +// to be one spelling yields no fingerprint at all rather than a vague one. +func TestSyntaxShowsNoDeprecatedSpelling(t *testing.T) { + clean := cleanCorpus() + var fps []fingerprint + for _, e := range deprecation.All() { + if fp, ok := deprecatedFingerprint(e, clean); ok { + fps = append(fps, fp) + t.Logf("%s fingerprint: %s", e.Code, fp) + } + } + // The derivation must keep producing patterns; if a registry reshuffle + // left it with almost nothing, this test would pass vacuously. + if len(fps) < 20 { + t.Fatalf("only %d registry entries yielded a fingerprint — the derivation has stopped working", len(fps)) + } + for _, f := range syntax.All() { + toks := lexMDL(stripComments(f.Syntax)) + for _, fp := range fps { + if syntaxDeprecationExempt[f.Path][fp.code] != "" { + continue + } + if at := fp.find(toks); at >= 0 { + t.Errorf("syntax topic %q, Syntax field, shows %s's deprecated spelling %q (matched %q) — write `%s`", + f.Path, fp.code, fp.old, joinTokens(toks[at:at+len(fp.pat)]), fp.canonical) + } + } + } +} + +// syntaxDeprecationExempt lists the topics whose Syntax legitimately writes a +// spelling that is deprecated elsewhere, each with the registry's own reason. +// Keep it to what the entry's Note names: an exemption stops the check for +// that code on the whole topic. +var syntaxDeprecationExempt = map[string]map[string]string{ + // MDL-DEPR001's Note: not reported for `create or replace translations`, + // which replaces the whole set — a different meaning, not an alias. + "translations": {deprecation.CreateOrReplace: "create or replace translations is not an alias"}, +} + +// TestDeprecatedFingerprintsMatchTheirOwnExample pins the derivation itself: +// a fingerprint that does not even find its entry's Example would let every +// Syntax text through, and one that finds the CanonicalExample would flag the +// spelling the docs are told to write. +func TestDeprecatedFingerprintsMatchTheirOwnExample(t *testing.T) { + for _, e := range deprecation.All() { + fp, ok := deprecatedFingerprint(e, cleanCorpus()) + if !ok { + continue + } + if fp.find(lexMDL(e.Example)) < 0 { + t.Errorf("%s: fingerprint %s does not find the entry's own Example %q", e.Code, fp, e.Example) + } + if fp.find(lexMDL(e.CanonicalExample)) >= 0 { + t.Errorf("%s: fingerprint %s finds the CanonicalExample %q", e.Code, fp, e.CanonicalExample) + } + } +} + +// cleanCorpus is MDL known to carry no deprecated spelling: every entry's +// CanonicalExample, and every syntax topic's Example, which TestExamplesParse +// holds to `check --deprecations=error`. A fingerprint that matches any of it +// is too vague to tell the deprecated spelling from the current one. +// Comments are dropped: prose may quote the old form to warn against it. +func cleanCorpus() [][]token { + var out [][]token + for _, e := range deprecation.All() { + out = append(out, lexMDL(e.CanonicalExample)) + } + for _, f := range syntax.All() { + if nonStatementTopics[f.Path] { + continue // prose, not held to the validator + } + out = append(out, lexMDL(stripComments(f.Example))) + } + return out +} + +type tokKind int + +const ( + tkWord tokKind = iota // keyword-shaped word: lower-case in the registry + tkIdent // identifier, qualified name, $variable, + tkString // '…' + tkNumber + tkPunct +) + +type token struct { + kind tokKind + text string // lower-cased +} + +var lexRe = regexp.MustCompile(`'(?:[^']|'')*'|\$[A-Za-z_]\w*|<[^<>\s][^<>]*>|[A-Za-z_][\w]*(?:\.[A-Za-z_]\w*)*|\d+(?:\.\d+)?|…|\.\.\.|\S`) + +// lexMDL splits MDL or metasyntax into tokens. Whether a word is a keyword or a +// name is not decidable from text alone; the registry's examples write +// keywords in lower case and names capitalised or qualified, and the +// fingerprint relies on that (see classify). +func lexMDL(s string) []token { + var out []token + for _, m := range lexRe.FindAllString(s, -1) { + // Square brackets are dropped on both sides: in a Syntax text they + // are the optional-group metasyntax, which splits a spelling apart + // (`NOT NULL [ERROR 'msg']`) without changing it. + if m == "[" || m == "]" { + continue + } + out = append(out, classify(m)) + } + return out +} + +func classify(m string) token { + low := strings.ToLower(m) + switch { + case strings.HasPrefix(m, "'"): + return token{tkString, low} + case strings.HasPrefix(m, "$"), strings.HasPrefix(m, "<") && len(m) > 1: + return token{tkIdent, low} + case m[0] >= '0' && m[0] <= '9': + return token{tkNumber, low} + case m[0] == '_' || (m[0]|0x20 >= 'a' && m[0]|0x20 <= 'z'): + if m != low || strings.Contains(m, ".") { + return token{tkIdent, low} + } + return token{tkWord, low} + } + return token{tkPunct, low} +} + +func stripComments(s string) string { + lines := strings.Split(s, "\n") + for i, l := range lines { + if j := strings.Index(l, "--"); j >= 0 { + lines[i] = l[:j] + } + } + return strings.Join(lines, "\n") +} + +type fingerprint struct { + code, old, canonical string + pat []token +} + +func (fp fingerprint) String() string { return "`" + joinTokens(fp.pat) + "`" } + +func joinTokens(ts []token) string { + parts := make([]string, len(ts)) + for i, t := range ts { + parts[i] = t.text + } + return strings.Join(parts, " ") +} + +// maxHunk bounds the changed span an entry may have and still be called one +// spelling. A structural rewrite that reorders a whole clause differs over a +// wider span; matching that span verbatim would find nothing useful, and +// shrinking it would find too much. +const maxHunk = 3 + +// maxContext bounds how many context tokens a fingerprint may grow by while it +// is being made specific enough (see deprecatedFingerprint). +const maxContext = 4 + +// deprecatedFingerprint derives the token pattern of e's deprecated spelling +// from the difference between its Example and CanonicalExample. +// +// The changed span alone is often a bare keyword (`snippet`, `remove`), so it +// starts with one token of context and then widens, a token at a time on +// alternating sides, until it is specific: it holds a keyword and matches no +// text in clean — MDL known to carry no deprecated spelling — and does not +// end on a name. A fingerprint that +// cannot get specific within maxContext tokens is dropped, never kept vague. +func deprecatedFingerprint(e deprecation.Entry, clean [][]token) (fingerprint, bool) { + if e.Example == "" || e.CanonicalExample == "" { + return fingerprint{}, false + } + old, can := lexMDL(e.Example), lexMDL(e.CanonicalExample) + p := 0 + for p < len(old) && p < len(can) && old[p].text == can[p].text { + p++ + } + q := 0 + for q < len(old)-p && q < len(can)-p && old[len(old)-1-q].text == can[len(can)-1-q].text { + q++ + } + lo, hi := p, len(old)-q + if hi-lo > maxHunk { + return fingerprint{}, false + } + hasWildcard := false + for _, t := range old[lo:hi] { + if t.kind == tkIdent || t.kind == tkString || t.kind == tkNumber { + hasWildcard = true + } + } + // An insertion, or a name/literal deprecated in place, says nothing until + // both neighbours say where. A changed keyword needs only one. + if lo == hi || hasWildcard { + if lo == 0 || hi == len(old) { + return fingerprint{}, false + } + lo, hi = lo-1, hi+1 + } else if lo > 0 { + lo-- + } else if hi < len(old) { + hi++ + } + canonical := [][]token{can} + specific := func(pat []token) bool { + keyword := false + for _, t := range pat { + if t.kind == tkWord { + keyword = true + } + } + if !keyword || len(pat) < 2 { + return false + } + // A name at the edge is a wildcard that any prose word satisfies + // (`Ready comment` would match "a comment"): anchor past it. + if pat[0].kind == tkIdent || pat[len(pat)-1].kind == tkIdent { + return false + } + probe := fingerprint{pat: pat} + for _, c := range append(canonical, clean...) { + if probe.find(c) >= 0 { + return false + } + } + return true + } + widen, preferPrev := 0, true + for !specific(old[lo:hi]) { + if widen == maxContext { + return fingerprint{}, false + } + switch { + case preferPrev && lo > 0, hi == len(old) && lo > 0: + lo-- + case hi < len(old): + hi++ + default: + return fingerprint{}, false + } + widen++ + preferPrev = !preferPrev + } + return fingerprint{code: e.Code, old: e.Old, canonical: e.Canonical, pat: old[lo:hi]}, true +} + +// find returns the index in ts where the fingerprint matches, or -1. A +// keyword or punctuation token matches itself (case-insensitively); an +// identifier matches any name-shaped token, a string any string, a number a +// number or a name (metasyntax writes `n`). +func (fp fingerprint) find(ts []token) int { + for i := 0; i+len(fp.pat) <= len(ts); i++ { + ok := true + for j, want := range fp.pat { + if !tokenMatches(want, ts[i+j]) { + ok = false + break + } + } + if ok { + return i + } + } + return -1 +} + +func tokenMatches(want, got token) bool { + switch want.kind { + case tkIdent: + return got.kind == tkIdent || got.kind == tkWord + case tkString: + return got.kind == tkString + case tkNumber: + return got.kind == tkNumber || got.kind == tkWord || got.kind == tkIdent + default: + return (got.kind == tkWord || got.kind == tkIdent || got.kind == tkPunct) && got.text == want.text + } +} diff --git a/cmd/mxcli/testrunner/generator_endpoint.go b/cmd/mxcli/testrunner/generator_endpoint.go index 2ccf2c9e8c..215a9ea7c8 100644 --- a/cmd/mxcli/testrunner/generator_endpoint.go +++ b/cmd/mxcli/testrunner/generator_endpoint.go @@ -168,10 +168,15 @@ func failVerdictExpr(exp Expect) string { // rewriteBodyForVerdict attaches an ON ERROR handler to every CALL in the test // body, turning a thrown error into a FAIL verdict and an early return. +// +// The verdict carries the raised message: a bare "exception during execution" +// left the user guessing, e.g. at an unresolvable [%CurrentUser%] token (tests +// run in a system context with no user). $latestError/Message is the same +// expression throwsHandler uses, measured there. func rewriteBodyForVerdict(lines []string, tc TestCase) []string { handler := []string{ - fmt.Sprintf(" SET $Verdict = '%s';", - escapeMDLString(verdictFailPrefix+"exception during execution")), + fmt.Sprintf(" SET $Verdict = '%s' + %s;", + escapeMDLString(verdictFailPrefix+"exception during execution: "), latestErrorMessage), " RETURN $Verdict;", } return attachOnError(lines, handler) diff --git a/cmd/mxcli/testrunner/generator_endpoint_test.go b/cmd/mxcli/testrunner/generator_endpoint_test.go index 1b695d3cf5..8521864719 100644 --- a/cmd/mxcli/testrunner/generator_endpoint_test.go +++ b/cmd/mxcli/testrunner/generator_endpoint_test.go @@ -102,6 +102,23 @@ func TestGenerateTestFlowsWrapsCallsWithErrorHandling(t *testing.T) { } } +// TestGenerateTestFlowsFailVerdictCarriesErrorMessage pins that a CALL which +// throws reports what was raised. The handler used to set the fixed text +// "FAIL:exception during execution" and discard the error, so a test failing on +// e.g. an unresolvable [%CurrentUser%] token (tests run in a system context) +// gave no hint of the cause. +func TestGenerateTestFlowsFailVerdictCarriesErrorMessage(t *testing.T) { + suite := &TestSuite{Tests: []TestCase{{ + ID: "test_1", Name: "throwing", MDL: "$r = CALL MICROFLOW Mod.A();", + }}} + mdl := GenerateTestFlows(suite) + + want := "SET $Verdict = '" + verdictFailPrefix + "exception during execution: ' + " + latestErrorMessage + ";" + if !strings.Contains(mdl, want) { + t.Errorf("the error handler does not carry $latestError/Message into the verdict; want %q in:\n%s", want, mdl) + } +} + // TestGenerateTestFlowsThrowsTestStartsFailed pins that an @throws test whose // body completes normally fails: the verdict is pre-set to a failure and only // the error handler clears it. diff --git a/cmd/mxcli/testrunner/localapp_options.go b/cmd/mxcli/testrunner/localapp_options.go index bb877c32a5..2a2a6b0f26 100644 --- a/cmd/mxcli/testrunner/localapp_options.go +++ b/cmd/mxcli/testrunner/localapp_options.go @@ -24,9 +24,9 @@ func localAppOptions(opts RunOptions, logPath string, env []string, w io.Writer) db := dbConfig(opts) return docker.LocalAppOptions{ ProjectPath: opts.ProjectPath, - AppPort: localTestAppPort, - AdminPort: localTestAdminPort, - ServePort: localTestServePort, + AppPort: opts.appPort(), + AdminPort: opts.adminPort(), + ServePort: opts.servePort(), // DeployDir is deliberately left at its default, /deployment. // It is shared with a concurrent `mxcli run --local` — unlike the ports and // the database — because mxbuild writes the deployment there and has no diff --git a/cmd/mxcli/testrunner/ports.go b/cmd/mxcli/testrunner/ports.go new file mode 100644 index 0000000000..7bda2cdca4 --- /dev/null +++ b/cmd/mxcli/testrunner/ports.go @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "fmt" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" +) + +// testPort is one of the three ports a --local test boot binds, with the flag +// that sets it — so every message about a port can say how to move it. +type testPort struct { + port int + role string + flag string +} + +// appPort, adminPort and servePort resolve the configured ports, falling back +// to the defaults the test boot has always used (which deliberately differ from +// `run --local`'s, so the two can run side by side). +func (o RunOptions) appPort() int { return portOr(o.AppPort, localTestAppPort) } +func (o RunOptions) adminPort() int { return portOr(o.AdminPort, localTestAdminPort) } +func (o RunOptions) servePort() int { return portOr(o.ServePort, localTestServePort) } + +func portOr(p, def int) int { + if p == 0 { + return def + } + return p +} + +// testPorts lists the ports a --local boot binds. The serve port is only bound +// when the run builds, so --skip-build leaves it out. +func (o RunOptions) testPorts() []testPort { + ports := []testPort{ + {o.appPort(), "app", "--app-port"}, + {o.adminPort(), "runtime admin API", "--admin-port"}, + } + if !o.SkipBuild { + ports = append(ports, testPort{o.servePort(), "mxbuild serve", "--serve-port"}) + } + return ports +} + +// validatePorts refuses port flags a run would ignore, and values that cannot +// work, before anything is built. +func validatePorts(opts RunOptions) error { + set := []testPort{ + {opts.AppPort, "app", "--app-port"}, + {opts.AdminPort, "runtime admin API", "--admin-port"}, + {opts.ServePort, "mxbuild serve", "--serve-port"}, + } + for _, p := range set { + if p.port == 0 { + continue + } + if !opts.Local || opts.Attach { + return fmt.Errorf("%s applies only to a --local test run "+ + "(--attach uses the ports of the app it attaches to; the Docker path configures the container's)", p.flag) + } + if p.port < 1 || p.port > 65535 { + return fmt.Errorf("%s %d is not a valid port (1-65535)", p.flag, p.port) + } + } + all := []testPort{ + {opts.appPort(), "app", "--app-port"}, + {opts.adminPort(), "runtime admin API", "--admin-port"}, + {opts.servePort(), "mxbuild serve", "--serve-port"}, + } + for i := range all { + for j := i + 1; j < len(all); j++ { + if all[i].port == all[j].port { + return fmt.Errorf("%s and %s resolve to the same port %d; each needs its own", + all[i].flag, all[j].flag, all[i].port) + } + } + } + return nil +} + +// checkTestPortsFree probes every port a --local boot will bind and fails on +// the first one already taken, naming the port and the flag that moves it. +// +// StartLocalApp has its own guard, but it runs only after the test endpoint has +// been injected into the project and cannot name `mxcli test`'s flags. Probing +// here fails before the project is touched, with a message the user can act on. +func checkTestPortsFree(opts RunOptions) error { + for _, p := range opts.testPorts() { + if err := docker.CheckPortFree(p.port, p.role, p.flag); err != nil { + return err + } + } + return nil +} diff --git a/cmd/mxcli/testrunner/ports_test.go b/cmd/mxcli/testrunner/ports_test.go new file mode 100644 index 0000000000..65e1c6d1a4 --- /dev/null +++ b/cmd/mxcli/testrunner/ports_test.go @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "fmt" + "net" + "strings" + "testing" +) + +// ChipCoV8 #14: `mxcli test --local` hard-coded 8081/8091/6544, so a machine +// with 8081 taken (a VS Code helper) could not run tests at all. The ports are +// now options, defaulting to the old values. + +func TestLocalAppOptions_DefaultPortsUnchanged(t *testing.T) { + o := localAppOptions(RunOptions{ProjectPath: "/x/App.mpr"}, "", nil, nil) + if o.AppPort != 8081 || o.AdminPort != 8091 || o.ServePort != 6544 { + t.Errorf("default ports = %d/%d/%d, want 8081/8091/6544", o.AppPort, o.AdminPort, o.ServePort) + } +} + +func TestLocalAppOptions_CarriesConfiguredPorts(t *testing.T) { + o := localAppOptions(RunOptions{ProjectPath: "/x/App.mpr", AppPort: 9081, AdminPort: 9091, ServePort: 7544}, "", nil, nil) + if o.AppPort != 9081 || o.AdminPort != 9091 || o.ServePort != 7544 { + t.Errorf("ports = %d/%d/%d, want 9081/9091/7544", o.AppPort, o.AdminPort, o.ServePort) + } +} + +func TestEndpointClient_UsesConfiguredAppPort(t *testing.T) { + c := testEndpointClient(RunOptions{AppPort: 9081}, "tok") + if !strings.HasPrefix(c.baseURL, "http://127.0.0.1:9081/") { + t.Errorf("baseURL = %q, want it on the configured app port 9081", c.baseURL) + } + // Control: the default is still the old port. + c = testEndpointClient(RunOptions{}, "tok") + if !strings.HasPrefix(c.baseURL, "http://127.0.0.1:8081/") { + t.Errorf("baseURL = %q, want the default app port 8081", c.baseURL) + } +} + +func TestValidateOptions_Ports(t *testing.T) { + tests := []struct { + name string + opts RunOptions + wantErr string + }{ + {name: "local with ports", opts: RunOptions{Local: true, AppPort: 9081, AdminPort: 9091, ServePort: 7544}}, + {name: "docker path refuses ports", opts: RunOptions{AppPort: 9081}, wantErr: "--app-port"}, + {name: "attach refuses ports", opts: RunOptions{Attach: true, AdminPort: 9091}, wantErr: "--admin-port"}, + {name: "out of range", opts: RunOptions{Local: true, ServePort: 70000}, wantErr: "--serve-port"}, + {name: "collision with a default", opts: RunOptions{Local: true, AdminPort: 8081}, wantErr: "same port"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateOptions(tt.opts) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("err = %v, want it to mention %q", err, tt.wantErr) + } + }) + } +} + +// The probe must fail fast — before the project is touched — and say which +// port, and which flag moves it. +func TestCheckTestPortsFree_NamesPortAndFlag(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + busy := ln.Addr().(*net.TCPAddr).Port + + free := freePort(t) + free2 := freePort(t) + + for _, tc := range []struct { + opts RunOptions + flag string + }{ + {RunOptions{Local: true, AppPort: busy, AdminPort: free, ServePort: free2}, "--app-port"}, + {RunOptions{Local: true, AppPort: free, AdminPort: busy, ServePort: free2}, "--admin-port"}, + {RunOptions{Local: true, AppPort: free, AdminPort: free2, ServePort: busy}, "--serve-port"}, + } { + err := checkTestPortsFree(tc.opts) + if err == nil { + t.Fatalf("%s: no error with port %d held", tc.flag, busy) + } + msg := err.Error() + if !strings.Contains(msg, fmt.Sprint(busy)) || !strings.Contains(msg, tc.flag) { + t.Errorf("%s: error does not name the port %d and the flag:\n%s", tc.flag, busy, msg) + } + } + + // Control: with nothing held the probe passes, so the error above is the + // listener's doing and not the probe failing unconditionally. + if err := checkTestPortsFree(RunOptions{Local: true, AppPort: free, AdminPort: free2, ServePort: freePort(t)}); err != nil { + t.Errorf("free ports reported busy: %v", err) + } + + // --skip-build starts no build server, so its port is not probed. + if err := checkTestPortsFree(RunOptions{Local: true, SkipBuild: true, AppPort: free, AdminPort: free2, ServePort: busy}); err != nil { + t.Errorf("--skip-build probed the serve port: %v", err) + } +} + +func freePort(t *testing.T) int { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + p := ln.Addr().(*net.TCPAddr).Port + ln.Close() + return p +} diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index c24a6c4f3e..0ab909a485 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -90,6 +90,14 @@ type RunOptions struct { // bundled mxbuild on macOS/Windows, the cached CDN download on Linux. MxBuildPath string + // AppPort, AdminPort and ServePort move the ports a --local run boots on — + // the app, the M2EE admin API and mxbuild --serve. Zero means the default + // (8081, 8091, 6544), chosen to sit beside `run --local`'s 8080/8090/6543. + // --attach and the Docker path refuse them (see validatePorts). + AppPort int + AdminPort int + ServePort int + // Timeout for runtime startup and test execution. Timeout time.Duration @@ -145,6 +153,14 @@ func Run(opts RunOptions) (*SuiteResult, error) { if err := validateOptions(opts); err != nil { return nil, err } + // Probe the ports before the project is touched: a taken port found after + // injection costs a build and a cleanup, and StartLocalApp's own guard cannot + // name the flag that moves it. + if opts.Local && !opts.Attach { + if err := checkTestPortsFree(opts); err != nil { + return nil, err + } + } timeout := opts.Timeout if timeout == 0 { @@ -224,6 +240,9 @@ func rejectVerifyOnLegacyRunner(suite *TestSuite) error { // says what to do instead. Watching depends on re-invoking tests without a // restart, which only the test endpoint can do. func validateOptions(opts RunOptions) error { + if err := validatePorts(opts); err != nil { + return err + } if opts.Attach { if opts.LegacyRunner { return fmt.Errorf("--attach cannot be combined with --legacy-runner: the after-startup runner can only run tests by restarting, which is what attaching avoids") diff --git a/cmd/mxcli/testrunner/runner_endpoint.go b/cmd/mxcli/testrunner/runner_endpoint.go index e80204045d..410fc85309 100644 --- a/cmd/mxcli/testrunner/runner_endpoint.go +++ b/cmd/mxcli/testrunner/runner_endpoint.go @@ -35,7 +35,7 @@ func bootForTests(opts RunOptions, token string, timeout time.Duration, w io.Wri return nil, fmt.Errorf("local runtime: %w", err) } - client := newEndpointClient(localTestAppPort, token) + client := testEndpointClient(opts, token) if err := client.waitReady(endpointReadyTimeout(timeout)); err != nil { app.Stop() return nil, fmt.Errorf("%w\n hint: check %s for a registration failure", err, logPath) @@ -43,6 +43,12 @@ func bootForTests(opts RunOptions, token string, timeout time.Duration, w io.Wri return &testAppSession{app: app, client: client, logPath: logPath}, nil } +// testEndpointClient is the client for the test endpoint of the app a --local +// run boots, on the app port the options select. +func testEndpointClient(opts RunOptions, token string) *endpointClient { + return newEndpointClient(opts.appPort(), token) +} + func (s *testAppSession) stop() { if s != nil && s.app != nil { s.app.Stop() diff --git a/docs-site/src/ide/syncing.md b/docs-site/src/ide/syncing.md index 776e5511a0..76c061b6d2 100644 --- a/docs-site/src/ide/syncing.md +++ b/docs-site/src/ide/syncing.md @@ -84,7 +84,9 @@ open a project stamped by a newer mxcli **without any warning**. For them the guard is the bootstrap script, `.claude/bootstrap-mxcli.sh`, which a newer `mxcli init` regenerates: before it links the `mxcli` on `PATH` into the project it compares that binary's version with the stamp, and it does not link -an older one — it downloads `MXCLI_TAG` (default `nightly`) instead. A +an older one — it downloads `MXCLI_TAG` instead. `MXCLI_TAG` defaults to the +release of the mxcli that generated the script (`vX.Y.Z`), or `nightly` when that +was a nightly or development build. A `./mxcli` older than the stamp is replaced the same way, through a temporary file, so a `./mxcli` that is a symlink to the `PATH` binary never has the download written through it. If the download fails, the older `./mxcli` is diff --git a/docs-site/src/tools/bootstrap-prompt.md b/docs-site/src/tools/bootstrap-prompt.md index e691a8ff4b..6523b57626 100644 --- a/docs-site/src/tools/bootstrap-prompt.md +++ b/docs-site/src/tools/bootstrap-prompt.md @@ -164,6 +164,11 @@ keep straight. file-driven and deterministic. After that, every new session runs the hook automatically; you never re-paste the prompt. Miss the script and the hook has nothing to run after a reap. +- **A failed hook does not block the session — so it says so.** The hook runs the + script under `|| true`. When `./mxcli run --local --setup --ensure-db` fails (for + example "no local PostgreSQL superuser available"), the script prints an + `ERROR: the app database was not provisioned: …` banner to stderr with the retry + command and the `--db-type hsqldb` alternative, and exits with the failure's status. - **mxcli delivery is an environment concern, not the prompt's.** The download in step 1 is the fragile part in a gated web session (a GitHub release `curl` may be blocked), and it is the one thing that cannot move into the skill — the skill is diff --git a/docs-site/src/tools/builtin-rules.md b/docs-site/src/tools/builtin-rules.md index 8a611b1c72..55beb7343b 100644 --- a/docs-site/src/tools/builtin-rules.md +++ b/docs-site/src/tools/builtin-rules.md @@ -29,6 +29,7 @@ The **lint rules** below run with `mxcli lint`. There is also a separate group o | **SEC001** | Entity access rules -- Ensures persistent entities have access rules defined | | **SEC002** | Password policy -- Checks for secure password configuration | | **SEC003** | Demo users -- Warns about demo users in production security level | +| **SEC010** | Unreadable page data -- A data grid, gallery, list view or combo box lists an entity that a user role allowed on the page cannot read (including its generalizations and specializations). At Production security it renders empty for that role and nothing else reports it. Info when the role reads only through an XPath-constrained rule; System entities use access measured from a deployed model, so a combo box over System.User gets its own message (it lists only the current user) | ## Convention Rules diff --git a/docs-site/src/tools/companion.md b/docs-site/src/tools/companion.md index 8e49bf7681..3cb3e45ee6 100644 --- a/docs-site/src/tools/companion.md +++ b/docs-site/src/tools/companion.md @@ -15,6 +15,11 @@ agent builds it. reports it. - **MDL source** — select a document to see its MDL, as `mxcli describe` prints it. Folders and other containers list what they hold instead. +- **Domain model diagrams** — select a module's *Domain Model* node in the desktop + app or VS Code to see its entities, attributes, associations and + generalizations, drawn where Studio Pro places them (`mxcli describe --format + svg domainmodel `). Hover a box for its attributes. The terminal draws + no pictures and lists the entities instead. In a wide pane (90 columns or more) the tree sits on the left and the source on the right, like a file explorer; each column pages on its own with its ▲ / ▼ diff --git a/docs-site/src/tools/linting.md b/docs-site/src/tools/linting.md index 959e9e70cd..8f7dc0eb95 100644 --- a/docs-site/src/tools/linting.md +++ b/docs-site/src/tools/linting.md @@ -17,7 +17,7 @@ The linting system provides: | Category | Prefix | Focus | |----------|--------|-------| | MDL | MDL001-MDL007 | Naming conventions, empty microflows, domain model size | -| Security | SEC001-SEC009 | Access rules, password policy, demo users, PII exposure | +| Security | SEC001-SEC010 | Access rules, password policy, demo users, PII exposure | | Convention | CONV001-CONV019 | Best practice conventions, error handling | | Quality | QUAL001-QUAL004 | Complexity, documentation, long microflows | | Architecture | ARCH001-ARCH003 | Cross-module data, entity business keys | diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 103443325e..5f37644a6a 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -9,8 +9,18 @@ and **Docker is only needed for the first**: - **Docker** — the container path. Requires a running Docker daemon. - **`--local`** — mxcli's own runtime, no daemon involved. It uses its own ports - (8081/8091) and its own `_test` database, so a `mxcli run --local` - dev loop can keep serving the same project while tests run. + (app 8081, admin 8091, mxbuild serve 6544) and its own `_test` + database, so a `mxcli run --local` dev loop can keep serving the same project + while tests run. + +Each port is probed before the project is touched. If one is taken — an editor +helper on 8081 is the usual suspect — the run stops with the port, what it was +for, the flag that moves it, and on Linux the process holding it. Move it with +the same flags `run --local` has (refused with `--attach` and on the Docker path): + +```bash +mxcli test tests/ -p app.mpr --local --app-port 9081 --admin-port 9091 --serve-port 7544 +``` `--local` provisions that scratch database on PostgreSQL by default. On a machine without PostgreSQL — where it would stop with *no local PostgreSQL superuser diff --git a/mdl/backend/widgetobj/builder.go b/mdl/backend/widgetobj/builder.go index b9d366a934..dd5de05857 100644 --- a/mdl/backend/widgetobj/builder.go +++ b/mdl/backend/widgetobj/builder.go @@ -322,6 +322,13 @@ func buildObjectListItemBSON(widgetID, listPropertyKey string, parentEntry pages } itemKind := detectObjectListItemKind(specByKey, item.ChildWidgets) + if itemKind == itemKindDefault && effectiveShowContentAs(specByKey, parentEntry) == "attribute" { + // No attribute bound, no content widgets — but showContentAs (set, or + // its widget default) still makes it an attribute column, and + // `mx update-widgets` gives it the attribute column's conventions + // (an empty tooltip ClientTemplate, not null — CE0463, ChipCoV8 #11). + itemKind = itemKindAttribute + } propsArr := bson.A{int32(2)} // Use template PropertyTypes order when available — Studio Pro expects @@ -405,6 +412,18 @@ func detectObjectListItemKind(specByKey map[string]backend.ObjectListItemPropert return itemKindDefault } +// effectiveShowContentAs returns an item's showContentAs value: the authored +// one, else the widget's default for it. "" when the item has no such property. +func effectiveShowContentAs(specByKey map[string]backend.ObjectListItemProperty, parentEntry pages.PropertyTypeIDEntry) string { + if sca, ok := specByKey["showContentAs"]; ok && sca.PrimitiveVal != "" { + return strings.ToLower(sca.PrimitiveVal) + } + if e, ok := parentEntry.NestedPropertyIDs["showContentAs"]; ok { + return strings.ToLower(e.DefaultValue) + } + return "" +} + // emptyClientTemplateRules maps (widgetID, listPropertyKey, itemKind, // propertyKey) tuples to "emit empty ClientTemplate" for unset TextTemplate // properties whose Studio Pro convention is an empty Forms$ClientTemplate diff --git a/mdl/backend/widgetobj/datagrid_column_empty_header_test.go b/mdl/backend/widgetobj/datagrid_column_empty_header_test.go new file mode 100644 index 0000000000..ed9b29f6ae --- /dev/null +++ b/mdl/backend/widgetobj/datagrid_column_empty_header_test.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ChipCoV8 #11: a Data grid 2 column with no caption, no attribute and no name +// had its `header` serialized as TextTemplate: null. `mx update-widgets` writes +// an empty Forms$ClientTemplate there, so `mx check` reported CE0463. The +// executor flags such a header EmptyTemplate; this test proves the flag +// reaches the encoded document through buildObjectListItemBSON. +package widgetobj + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/sdk/pages" + "go.mongodb.org/mongo-driver/bson" +) + +const ( + dgColumnHeaderID = "00000000000000000000000000000011" + dgColumnShowID = "00000000000000000000000000000012" +) + +func dgColumnEntry() pages.PropertyTypeIDEntry { + return pages.PropertyTypeIDEntry{ + PropertyTypeID: "00000000000000000000000000000010", + NestedKeyOrder: []string{"showContentAs", "header"}, + NestedPropertyIDs: map[string]pages.PropertyTypeIDEntry{ + "showContentAs": {PropertyTypeID: dgColumnShowID, ValueType: "Enumeration", DefaultValue: "attribute"}, + // The column header is an optional TextTemplate with no default. + "header": {PropertyTypeID: dgColumnHeaderID, ValueType: "TextTemplate"}, + }, + } +} + +func headerTemplate(t *testing.T, spec backend.ObjectListItemSpec) any { + t.Helper() + got := buildObjectListItemBSON("com.mendix.widget.web.datagrid.Datagrid", "columns", dgColumnEntry(), spec) + return templateOf(t, got, dgColumnHeaderID) +} + +func TestDataGridColumn_EmptyTemplateHeaderEncodesEmptyClientTemplate(t *testing.T) { + spec := backend.ObjectListItemSpec{Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + {PropertyKey: "header", Operation: "texttemplate", EmptyTemplate: true}, + }} + tt, ok := headerTemplate(t, spec).(bson.D) + if !ok { + t.Fatalf("header TextTemplate = %v, want an empty Forms$ClientTemplate (null is CE0463)", headerTemplate(t, spec)) + } + m := map[string]any{} + for _, e := range tt { + m[e.Key] = e.Value + } + if m["$Type"] != "Forms$ClientTemplate" { + t.Fatalf("header TextTemplate $Type = %v, want Forms$ClientTemplate", m["$Type"]) + } + // update-widgets' exact shape: no params, no translations in either text. + if p, _ := m["Parameters"].(bson.A); len(p) != 1 { + t.Errorf("Parameters = %v, want [2] only", m["Parameters"]) + } + for _, k := range []string{"Template", "Fallback"} { + var items bson.A + for _, e := range m[k].(bson.D) { + if e.Key == "Items" { + items, _ = e.Value.(bson.A) + } + } + if len(items) != 1 { + t.Errorf("%s.Items = %v, want [3] only (no translations)", k, items) + } + } + + // Control: the same header without the flag stays null — the pre-fix + // output, and the convention for an unflagged optional TextTemplate. + spec.Properties[1].EmptyTemplate = false + if tt := headerTemplate(t, spec); tt != nil { + t.Errorf("unflagged empty header = %v, want null", tt) + } +} diff --git a/mdl/backend/widgetobj/datagrid_column_unbound_tooltip_test.go b/mdl/backend/widgetobj/datagrid_column_unbound_tooltip_test.go new file mode 100644 index 0000000000..7877972245 --- /dev/null +++ b/mdl/backend/widgetobj/datagrid_column_unbound_tooltip_test.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ChipCoV8 #11, second half: a column with an empty caption — no attribute, no +// content widgets, no ShowContentAs — is still an ATTRIBUTE column (that is +// showContentAs' default), and `mx update-widgets` writes its `tooltip` as an +// empty Forms$ClientTemplate. Classified as itemKindDefault it got null, and +// `mx check --no-update-widgets` reported CE0463 on the grid. +package widgetobj + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +const dgColumnTooltipID = "00000000000000000000000000000013" + +func dgColumnEntryWithTooltip() pages.PropertyTypeIDEntry { + e := dgColumnEntry() + e.NestedKeyOrder = append(e.NestedKeyOrder, "tooltip") + e.NestedPropertyIDs["tooltip"] = pages.PropertyTypeIDEntry{PropertyTypeID: dgColumnTooltipID, ValueType: "TextTemplate"} + return e +} + +func tooltipTemplate(t *testing.T, spec backend.ObjectListItemSpec) any { + t.Helper() + got := buildObjectListItemBSON("com.mendix.widget.web.datagrid.Datagrid", "columns", dgColumnEntryWithTooltip(), spec) + return templateOf(t, got, dgColumnTooltipID) +} + +func TestDataGridColumn_UnboundAttributeColumnGetsEmptyTooltip(t *testing.T) { + // No showContentAs in the spec: the widget default ("attribute") applies. + if tt := tooltipTemplate(t, backend.ObjectListItemSpec{}); tt == nil { + t.Error("tooltip of an attribute-less column with default showContentAs is null; update-widgets writes an empty ClientTemplate (CE0463)") + } + // Explicit ShowContentAs: attribute, still no attribute bound. + explicit := backend.ObjectListItemSpec{Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "attribute"}, + }} + if tt := tooltipTemplate(t, explicit); tt == nil { + t.Error("tooltip of an explicit showContentAs: attribute column with no attribute is null; want an empty ClientTemplate") + } + // Control: a custom-content column keeps tooltip null (Studio Pro's convention). + custom := backend.ObjectListItemSpec{Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + }} + if tt := tooltipTemplate(t, custom); tt != nil { + t.Errorf("custom-content column tooltip = %v, want null", tt) + } +} diff --git a/mdl/executor/cmd_alter_flow.go b/mdl/executor/cmd_alter_flow.go index 1fff277ba8..29f360fdfa 100644 --- a/mdl/executor/cmd_alter_flow.go +++ b/mdl/executor/cmd_alter_flow.go @@ -471,6 +471,7 @@ func (a *alterFlowContext) fragmentBuilder(ctx *ExecContext) *flowBuilder { hierarchy: hierarchy, restServices: restServices, isNanoflow: a.stmt.Nanoflow, + scriptFlows: ctx.scriptFlows, // A fragment is spliced into a stored flow: a member it cannot // qualify is refused, never written bare (ako/mxcli#885). diff --git a/mdl/executor/cmd_domainmodel_svg.go b/mdl/executor/cmd_domainmodel_svg.go new file mode 100644 index 0000000000..5ccfab4813 --- /dev/null +++ b/mdl/executor/cmd_domainmodel_svg.go @@ -0,0 +1,406 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "fmt" + "html" + "math" + "strings" + + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// cmd_domainmodel_svg.go draws a module's domain model as a finished SVG, for +// clients with no layout engine of their own (the Claude Code companion mod has +// no DOM, so the ELK graph the VS Code extension lays out in a browser is no +// use to it). No layout is computed: entities are drawn where the model stores +// them, which is where Studio Pro shows them. + +// Palette, per entity kind (classifyEntity), close to Studio Pro's. +var svgEntityHeader = map[string]string{ + "persistent": "#2B5FD9", + "nonpersistent": "#E07B2E", + "external": "#8A4FD6", + "view": "#2E9E6A", +} + +const ( + svgPad = 24.0 + svgHeaderH = 24.0 + svgExtendsH = 12.0 + svgLineH = 16.0 + svgTextPad = 16.0 + svgMinW = 90.0 + svgMinGap = 8.0 + svgFont = `font-family="Segoe UI,Helvetica Neue,Arial,sans-serif"` + svgMaxNameLen = 60 + // svgMaxSpread caps how far the stored layout is pulled apart to fit full + // names, so one outlier cannot blow a diagram up to several screens. + svgMaxSpread = 2.5 +) + +// textPx estimates a string's width in a proportional sans-serif at `size` px. +// Studio Pro draws entities in such a font and places them for its own box +// widths; sizing for a monospace (as the ELK graph does) made boxes wider than +// the room Studio Pro left, and they overlapped. +func textPx(s string, size float64, bold bool) float64 { + em := 0.0 + for _, r := range s { + switch { + case strings.ContainsRune("il.,:;'|!tfjrI ()[]", r): + em += 0.32 + case strings.ContainsRune("mwMW", r): + em += 0.86 + case r >= 'A' && r <= 'Z': + em += 0.66 + case r >= '0' && r <= '9': + em += 0.56 + default: + em += 0.54 + } + } + if bold { + em *= 1.06 + } + return em * size +} + +type svgBox struct { + x, y, w, h float64 +} + +func (b svgBox) center() (float64, float64) { return b.x + b.w/2, b.y + b.h/2 } + +// edgePoint is where the line from the box's centre towards (tx, ty) leaves the box. +func (b svgBox) edgePoint(tx, ty float64) (float64, float64) { + cx, cy := b.center() + dx, dy := tx-cx, ty-cy + if dx == 0 && dy == 0 { + return cx, cy + } + scale := math.Inf(1) + if dx != 0 { + scale = math.Min(scale, (b.w/2)/math.Abs(dx)) + } + if dy != 0 { + scale = math.Min(scale, (b.h/2)/math.Abs(dy)) + } + return cx + dx*scale, cy + dy*scale +} + +// edgePointFrom is where the line from (sx, sy) — a point inside the box, +// shifted off its centre for a parallel association — towards (tx, ty) leaves +// the box. +func (b svgBox) edgePointFrom(sx, sy, tx, ty float64) (float64, float64) { + dx, dy := tx-sx, ty-sy + if dx == 0 && dy == 0 { + return sx, sy + } + scale := math.Inf(1) + if dx > 0 { + scale = math.Min(scale, (b.x+b.w-sx)/dx) + } else if dx < 0 { + scale = math.Min(scale, (b.x-sx)/dx) + } + if dy > 0 { + scale = math.Min(scale, (b.y+b.h-sy)/dy) + } else if dy < 0 { + scale = math.Min(scale, (b.y-sy)/dy) + } + if scale < 0 || math.IsInf(scale, 1) { + return sx, sy + } + return sx + dx*scale, sy + dy*scale +} + +func svgNum(f float64) string { return fmt.Sprintf("%.0f", f) } + +func svgText(s string) string { + if len(s) > svgMaxNameLen { + s = s[:svgMaxNameLen-1] + "…" + } + return html.EscapeString(s) +} + +// renderDomainModelSVG draws the entities, the associations between them and +// the generalizations of one module. An association or generalization whose +// other end is outside the module has no position to draw to: the association +// is left out, the generalization is written under the entity's name. +func renderDomainModelSVG(moduleName string, entities []*domainmodel.Entity, assocs []*domainmodel.Association) string { + if len(entities) == 0 { + return `` + + `` + + svgText(moduleName) + ` has no entities` + } + + boxes := map[model.ID]svgBox{} + byName := map[string]model.ID{} + for _, e := range entities { + w := textPx(e.Name, 12, true) + if e.GeneralizationRef != "" { + w = math.Max(w, textPx("extends "+e.GeneralizationRef, 10, false)) + } + for _, at := range e.Attributes { + w = math.Max(w, textPx(at.Name+": "+at.Type.GetTypeName(), 11, false)) + } + h := svgHeaderH + svgLineH*math.Max(1, float64(len(e.Attributes))) + 4 + if e.GeneralizationRef != "" { + h += svgExtendsH + } + boxes[e.ID] = svgBox{x: float64(e.Location.X), y: float64(e.Location.Y), w: math.Max(svgMinW, w+svgTextPad), h: h} + byName[moduleName+"."+e.Name] = e.ID + } + // Studio Pro draws entities at a fixed width and cuts long names off, a + // common complaint. Keep its arrangement, but pull it apart (x first, then + // y at the new x) just enough that every box fits its longest line, up to + // svgMaxSpread; past that the clamp below takes over. + spread(entities, boxes, true) + spread(entities, boxes, false) + + // A box never runs into the entity to its right, nor into the one below: + // widths first (against neighbours sharing its rows), then heights (against + // neighbours sharing its columns, at the clamped width). The text is + // clipped to the box; the tooltip keeps all of it. + clamped := map[model.ID]bool{} + for _, e := range entities { + a := boxes[e.ID] + for _, o := range entities { + b := boxes[o.ID] + if o.ID == e.ID || b.x <= a.x || a.y >= b.y+b.h || b.y >= a.y+a.h { + continue + } + if room := b.x - a.x - svgMinGap; room < a.w { + a.w, clamped[e.ID] = math.Max(60, room), true + } + } + boxes[e.ID] = a + } + for _, e := range entities { + a := boxes[e.ID] + for _, o := range entities { + b := boxes[o.ID] + if o.ID == e.ID || b.y <= a.y || a.x >= b.x+b.w || b.x >= a.x+a.w { + continue + } + if room := b.y - a.y - svgMinGap; room < a.h { + a.h, clamped[e.ID] = math.Max(svgHeaderH, room), true + } + } + boxes[e.ID] = a + } + minX, minY := math.Inf(1), math.Inf(1) + maxX, maxY := math.Inf(-1), math.Inf(-1) + for _, b := range boxes { + minX, minY = math.Min(minX, b.x), math.Min(minY, b.y) + maxX, maxY = math.Max(maxX, b.x+b.w), math.Max(maxY, b.y+b.h) + } + minX, minY, maxX, maxY = minX-svgPad, minY-svgPad, maxX+svgPad, maxY+svgPad + w, h := maxX-minX, maxY-minY + + var sb strings.Builder + fmt.Fprintf(&sb, ``, + svgNum(minX), svgNum(minY), svgNum(w), svgNum(h), svgNum(w), svgNum(h), svgFont) + fmt.Fprintf(&sb, ``, svgNum(minX), svgNum(minY), svgNum(w), svgNum(h)) + + // Associations between the same two entities are spread side by side, so + // their lines and labels do not print over one another. + pairKey := func(a *domainmodel.Association) string { + if a.ParentID < a.ChildID { + return string(a.ParentID) + "|" + string(a.ChildID) + } + return string(a.ChildID) + "|" + string(a.ParentID) + } + pairTotal := map[string]int{} + for _, a := range assocs { + pairTotal[pairKey(a)]++ + } + pairSeen := map[string]int{} + + // Lines first, so the boxes sit on top of them. + for _, a := range assocs { + from, okFrom := boxes[a.ParentID] // ParentPointer is the FROM side: it owns the reference + to, okTo := boxes[a.ChildID] + if !okFrom || !okTo { + continue + } + key := pairKey(a) + offset := (float64(pairSeen[key]) - float64(pairTotal[key]-1)/2) * 16 + pairSeen[key]++ + tcx, tcy := to.center() + fcx, fcy := from.center() + // Shift both aim points perpendicular to the centre line by the offset. + if l := math.Hypot(tcx-fcx, tcy-fcy); l > 0 && offset != 0 { + px, py := -(tcy-fcy)/l*offset, (tcx-fcx)/l*offset + tcx, tcy, fcx, fcy = tcx+px, tcy+py, fcx+px, fcy+py + } + x1, y1 := from.edgePointFrom(fcx, fcy, tcx, tcy) + x2, y2 := to.edgePointFrom(tcx, tcy, fcx, fcy) + fromMult, toMult := "*", "1" + if a.Type == domainmodel.AssociationTypeReferenceSet { + toMult = "*" + } + fmt.Fprintf(&sb, `%s`, svgText(a.Name)) + fmt.Fprintf(&sb, ``, svgNum(x1), svgNum(y1), svgNum(x2), svgNum(y2)) + writeEndLabel(&sb, x1, y1, x2, y2, fromMult) + writeEndLabel(&sb, x2, y2, x1, y1, toMult) + // Labels of parallel associations stack, one line each. + fmt.Fprintf(&sb, `%s`, + svgNum((x1+x2)/2), svgNum((y1+y2)/2-4+offset/16*12), svgText(a.Name)) + sb.WriteString(``) + } + for _, e := range entities { + parentID, ok := byName[e.GeneralizationRef] + if !ok { + continue + } + child, parent := boxes[e.ID], boxes[parentID] + pcx, pcy := parent.center() + ccx, ccy := child.center() + x1, y1 := child.edgePoint(pcx, pcy) + x2, y2 := parent.edgePoint(ccx, ccy) + fmt.Fprintf(&sb, ``, svgNum(x1), svgNum(y1), svgNum(x2), svgNum(y2)) + writeArrowHead(&sb, x1, y1, x2, y2) + sb.WriteString(``) + } + + for i, e := range entities { + b := boxes[e.ID] + kind := classifyEntity(e) + var tip strings.Builder + tip.WriteString(e.Name) + if e.GeneralizationRef != "" { + fmt.Fprintf(&tip, " (extends %s)", e.GeneralizationRef) + } + for _, at := range e.Attributes { + fmt.Fprintf(&tip, "\n%s: %s", at.Name, at.Type.GetTypeName()) + } + fmt.Fprintf(&sb, `%s`, kind, svgNum(b.x), svgNum(b.y), html.EscapeString(tip.String())) + headerH := svgHeaderH + if e.GeneralizationRef != "" { + headerH += svgExtendsH + } + headerH = math.Min(headerH, b.h) + fmt.Fprintf(&sb, ``, svgNum(b.w), svgNum(b.h)) + fmt.Fprintf(&sb, ``, + svgNum(b.w-4), svgNum(b.w), svgNum(headerH), svgEntityHeader[kind]) + textOpen := "" + if clamped[e.ID] { + fmt.Fprintf(&sb, ``, i, svgNum(b.w-4), svgNum(b.h)) + textOpen = fmt.Sprintf(``, i) + } + sb.WriteString(textOpen) + fmt.Fprintf(&sb, `%s`, svgText(e.Name)) + if e.GeneralizationRef != "" { + fmt.Fprintf(&sb, `extends %s`, svgText(e.GeneralizationRef)) + } + for j, at := range e.Attributes { + fmt.Fprintf(&sb, `%s`, + svgNum(headerH+svgLineH*float64(j)+13), svgText(at.Name+": "+at.Type.GetTypeName())) + } + sb.WriteString(``) + } + sb.WriteString(``) + return sb.String() +} + +// spread scales the boxes' positions along one axis, from the smallest +// coordinate, by the least factor (1 to svgMaxSpread) that leaves every box +// clear of the next one along that axis among those it shares the other axis +// with. +func spread(entities []*domainmodel.Entity, boxes map[model.ID]svgBox, horizontal bool) { + k := 1.0 + origin := math.Inf(1) + for _, e := range entities { + a := boxes[e.ID] + if horizontal { + origin = math.Min(origin, a.x) + } else { + origin = math.Min(origin, a.y) + } + for _, o := range entities { + b := boxes[o.ID] + if o.ID == e.ID { + continue + } + var gap, need float64 + if horizontal { + if b.x <= a.x || a.y >= b.y+b.h || b.y >= a.y+a.h { + continue + } + gap, need = b.x-a.x, a.w+svgMinGap + } else { + if b.y <= a.y || a.x >= b.x+b.w || b.x >= a.x+a.w { + continue + } + gap, need = b.y-a.y, a.h+svgMinGap + } + if need > gap { + k = math.Max(k, need/gap) + } + } + } + k = math.Min(k, svgMaxSpread) + if k == 1 { + return + } + for id, b := range boxes { + if horizontal { + b.x = origin + (b.x-origin)*k + } else { + b.y = origin + (b.y-origin)*k + } + boxes[id] = b + } +} + +// writeEndLabel puts a multiplicity just inside the line's end at (x, y), set +// off to one side so it does not sit on the line. +func writeEndLabel(sb *strings.Builder, x, y, ox, oy float64, label string) { + dx, dy := ox-x, oy-y + l := math.Hypot(dx, dy) + if l == 0 { + return + } + ux, uy := dx/l, dy/l + lx, ly := x+ux*12-uy*8, y+uy*12+ux*8 + fmt.Fprintf(sb, `%s`, svgNum(lx), svgNum(ly), label) +} + +// writeArrowHead draws a hollow triangle at (x2, y2), pointing along the line. +func writeArrowHead(sb *strings.Builder, x1, y1, x2, y2 float64) { + dx, dy := x2-x1, y2-y1 + l := math.Hypot(dx, dy) + if l == 0 { + return + } + ux, uy := dx/l, dy/l + bx, by := x2-ux*12, y2-uy*12 + fmt.Fprintf(sb, ``, + svgNum(x2), svgNum(y2), svgNum(bx-uy*7), svgNum(by+ux*7), svgNum(bx+uy*7), svgNum(by-ux*7)) +} + +// domainModelSVG writes the SVG of a module's domain model. +func domainModelSVG(ctx *ExecContext, moduleName string) error { + if !ctx.Connected() { + return mdlerrors.NewNotConnected() + } + module, err := findModule(ctx, moduleName) + if err != nil { + return err + } + dm, err := ctx.Backend.GetDomainModel(module.ID) + if err != nil { + return mdlerrors.NewBackend("get domain model", err) + } + fmt.Fprint(ctx.Output, renderDomainModelSVG(moduleName, dm.Entities, dm.Associations)) + return nil +} + +// DomainModelSVG is the exported Executor method, called from outside the package. +func (e *Executor) DomainModelSVG(moduleName string) error { + return domainModelSVG(e.newExecContext(context.Background()), moduleName) +} diff --git a/mdl/executor/cmd_domainmodel_svg_test.go b/mdl/executor/cmd_domainmodel_svg_test.go new file mode 100644 index 0000000000..25495d7e09 --- /dev/null +++ b/mdl/executor/cmd_domainmodel_svg_test.go @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "encoding/xml" + "fmt" + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +var regexpLines = regexp.MustCompile(`[^<]*(]*>)`) + +func svgTestModel() ([]*domainmodel.Entity, []*domainmodel.Association) { + customer := &domainmodel.Entity{Name: "Customer", Persistable: true, Location: model.Point{X: 100, Y: 100}} + customer.ID = "c" + customer.Attributes = []*domainmodel.Attribute{{Name: "Name", Type: &domainmodel.StringAttributeType{Length: 200}}} + order := &domainmodel.Entity{Name: "Order", Persistable: true, Location: model.Point{X: 400, Y: 120}} + order.ID = "o" + filter := &domainmodel.Entity{Name: "OrderFilter", Persistable: false, Location: model.Point{X: 100, Y: 300}} + filter.ID = "f" + account := &domainmodel.Entity{Name: "Account", Persistable: true, Location: model.Point{X: 400, Y: 300}, GeneralizationRef: "System.User"} + account.ID = "a" + // Order_Customer: from Order (owns the reference) to Customer. + assoc := &domainmodel.Association{Name: "Order_Customer", ParentID: "o", ChildID: "c", Type: domainmodel.AssociationTypeReference} + tags := &domainmodel.Association{Name: "Order_Tag", ParentID: "o", ChildID: "elsewhere", Type: domainmodel.AssociationTypeReferenceSet} + return []*domainmodel.Entity{customer, order, filter, account}, []*domainmodel.Association{assoc, tags} +} + +// The SVG has to be well-formed: the mod hands it to the surface as is, and a +// broken document draws nothing. +func TestDomainModelSVG_IsWellFormed(t *testing.T) { + entities, assocs := svgTestModel() + svg := renderDomainModelSVG("Sales", entities, assocs) + dec := xml.NewDecoder(strings.NewReader(svg)) + for { + _, err := dec.Token() + if err != nil { + if err.Error() == "EOF" { + break + } + t.Fatalf("not well-formed XML: %v\n%s", err, svg) + } + } + if !strings.HasPrefix(svg, " root with a viewBox, got %.80s", svg) + } +} + +// Entities sit where Studio Pro put them, with their attributes, and the box +// colour says what kind of entity it is. +func TestDomainModelSVG_DrawsEntitiesAtTheirStoredPositions(t *testing.T) { + entities, assocs := svgTestModel() + svg := renderDomainModelSVG("Sales", entities, assocs) + for _, want := range []string{ + ``, + ``, + ``, + ">Customer<", ">Name: String<", ">OrderFilter<", + ">extends System.User<", + } { + if !strings.Contains(svg, want) { + t.Errorf("missing %q in\n%s", want, svg) + } + } +} + +// An association between two drawn entities is a line with its multiplicities; +// one to an entity of another module has nowhere to go and is left out rather +// than drawn to the origin. +func TestDomainModelSVG_DrawsAssociationsBetweenDrawnEntities(t *testing.T) { + entities, assocs := svgTestModel() + svg := renderDomainModelSVG("Sales", entities, assocs) + if strings.Count(svg, `class="association"`) != 1 { + t.Errorf("want exactly one association drawn (Order_Customer), got\n%s", svg) + } + if !strings.Contains(svg, "Order_Customer") { + t.Error("the association's name is not in the drawing") + } + if strings.Contains(svg, "Order_Tag") { + t.Error("an association to an entity outside the module was drawn") + } +} + +func TestDomainModelSVG_EscapesNames(t *testing.T) { + e := &domainmodel.Entity{Name: "A&C", Persistable: true} + e.ID = "x" + svg := renderDomainModelSVG("M", []*domainmodel.Entity{e}, nil) + if strings.Contains(svg, "A") || !strings.Contains(svg, "A<B>&C") { + t.Errorf("names are not escaped:\n%s", svg) + } +} + +func TestDomainModelSVG_EmptyModuleSaysSo(t *testing.T) { + svg := renderDomainModelSVG("Empty", nil, nil) + if !strings.Contains(svg, "no entities") { + t.Errorf("an empty domain model should say so, got\n%s", svg) + } +} + +// Two associations between the same pair of entities are drawn side by side, +// not on top of each other (MicroflowSelection_Microflow and _Active in a real +// model printed their labels over one another). +func TestDomainModelSVG_SeparatesParallelAssociations(t *testing.T) { + entities, _ := svgTestModel() + assocs := []*domainmodel.Association{ + {Name: "Order_Customer", ParentID: "o", ChildID: "c", Type: domainmodel.AssociationTypeReference}, + {Name: "Order_Customer_Billing", ParentID: "o", ChildID: "c", Type: domainmodel.AssociationTypeReference}, + } + svg := renderDomainModelSVG("Sales", entities, assocs) + m := regexpLines.FindAllStringSubmatch(svg, -1) + if len(m) < 2 { + t.Fatalf("want two association lines, got %d", len(m)) + } + if m[0][1] == m[1][1] { + t.Errorf("parallel associations drawn on the same line: %s", m[0][1]) + } +} + +var regexpEntity = regexp.MustCompile(`[^<]* 0 { - return nil + if errs := fb.GetErrors(); len(errs) > 0 { + return nil, buildDependency(strings.Join(errs, "\n")) } } _, err = a.plan(ctx) if err == nil { - return nil + return nil, "" } var nf *mdlerrors.NotFoundError if errors.As(err, &nf) { - return nil + return nil, "" } return []linter.Violation{{ RuleID: alterFlowRefusedRule, Severity: linter.SeverityError, Message: "exec would refuse this statement and write nothing: " + err.Error(), Location: linter.Location{Module: s.Name.Module, DocumentType: s.Kind(), DocumentName: s.Name.Name}, - }} + }}, "" +} + +// FormatUnpredictedFlows is the note check and exec's preflight print for +// flow changes whose verdict could not be predicted, one per line: +// +// N flow change(s) not predicted — exec decides when it reaches them: +// microflow M.N — depends on: +// +// "" when there are none. +func FormatUnpredictedFlows(us []UnpredictedFlow) string { + if len(us) == 0 { + return "" + } + var b strings.Builder + fmt.Fprintf(&b, "Note: %d flow change(s) not predicted — exec decides when it reaches them, after the statements before have been written:\n", len(us)) + for _, u := range us { + fmt.Fprintf(&b, " %s %s — depends on: %s\n", u.Kind, u.Name, u.Reason) + } + return b.String() } diff --git a/mdl/executor/validate_flow_verdicts_script_flows_pedapp_test.go b/mdl/executor/validate_flow_verdicts_script_flows_pedapp_test.go new file mode 100644 index 0000000000..329f0823e3 --- /dev/null +++ b/mdl/executor/validate_flow_verdicts_script_flows_pedapp_test.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// A `create or modify` whose new body calls a flow an earlier statement of the +// same script creates did not build against the stored project, so check -p +// left it unpredicted and said "no flow change exec would refuse" — and exec's +// preflight said nothing either, so exec wrote the earlier statements and then +// refused the flow, leaving the script half-applied. The verdict build now +// resolves calls to flows the script itself declares. + +// verdictActStored is the flow as stored: a retrieve whose output a later +// change reads. Moving both into a sub-flow and calling it replaces the +// retrieve while the change still reads $Reporter, which the splice refuses. +const verdictActStored = `mdl 1; +create microflow MyFirstModule.VerdictAct ($F: FeedbackModule.Feedback) +begin + retrieve $Reporter from System.User first; + change $F (Subject = $Reporter/Name); + log info node 'X' 'done'; +end;` + +const verdictSubCreate = `create microflow MyFirstModule.%s ($F: FeedbackModule.Feedback) +begin + retrieve $Reporter from System.User first; + change $F (Subject = $Reporter/Name); +end;` + +func verdictActCallingSub(sub string) string { + return `create or modify microflow MyFirstModule.VerdictAct ($F: FeedbackModule.Feedback) +begin + call microflow MyFirstModule.` + sub + ` (F = $F); + log info node 'X' 'done'; +end;` +} + +func verdictReport(t *testing.T, exec *Executor, src string) FlowVerdictReport { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v\n%s", errs[0], src) + } + return exec.CheckFlowVerdictsReport(prog) +} + +func verdictRefusals(vs []linter.Violation) string { + var says []string + for _, v := range vs { + if v.RuleID == FlowRebuildRule && v.Severity == linter.SeverityError { + says = append(says, v.Message) + } + } + return strings.Join(says, "\n") +} + +const stillUsed = "is still used by" + +func TestCheckFlowVerdicts_CallToFlowTheScriptCreates(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + if err := agreeExec(t, exec, verdictActStored); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + if err := agreeExec(t, exec, "mdl 1;\n"+strings.Replace(verdictSubCreate, "%s", "VerdictStoredSub", 1)); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + + // Control: the callee is already stored, so the body builds and the + // refusal is predicted — this already worked. + t.Run("control: callee stored", func(t *testing.T) { + r := verdictReport(t, exec, "mdl 1;\n"+verdictActCallingSub("VerdictStoredSub")) + if got := verdictRefusals(r.Violations); !strings.Contains(got, stillUsed) { + t.Fatalf("check must predict the refusal %q, reported: %v", stillUsed, r.Violations) + } + if len(r.Unpredicted) != 0 { + t.Errorf("nothing is unpredicted here, reported: %+v", r.Unpredicted) + } + }) + + // The reported case: the callee is created by the statement before. + t.Run("callee created earlier in the script", func(t *testing.T) { + src := "mdl 1;\n" + strings.Replace(verdictSubCreate, "%s", "VerdictNewSub", 1) + "\n" + verdictActCallingSub("VerdictNewSub") + r := verdictReport(t, exec, src) + if got := verdictRefusals(r.Violations); !strings.Contains(got, stillUsed) { + t.Fatalf("check must predict exec's refusal %q, reported violations %v, unpredicted %+v", + stillUsed, r.Violations, r.Unpredicted) + } + if len(r.Unpredicted) != 0 { + t.Errorf("the change is predicted, yet reported unpredicted: %+v", r.Unpredicted) + } + // exec agrees: it writes the sub-flow and then refuses VerdictAct. + err := agreeExec(t, exec, src) + if err == nil || !strings.Contains(err.Error(), stillUsed) { + t.Fatalf("exec must refuse with %q, got: %v", stillUsed, err) + } + }) + + // A body that depends on something else an earlier statement makes — + // here a flow a rename brings into being, which has no declaration to + // resolve to — is still not predicted, and is now reported as such + // instead of passing silently. + t.Run("unpredictable change is reported", func(t *testing.T) { + src := "mdl 1;\nrename microflow MyFirstModule.VerdictStoredSub to VerdictRenamedSub;\n" + + verdictActCallingSub("VerdictRenamedSub") + r := verdictReport(t, exec, src) + if len(r.Unpredicted) != 1 || r.Unpredicted[0].Name != "MyFirstModule.VerdictAct" { + t.Fatalf("want VerdictAct reported unpredicted, got %+v (violations %v)", r.Unpredicted, r.Violations) + } + if !strings.Contains(r.Unpredicted[0].Reason, "VerdictRenamedSub") { + t.Errorf("the reason must name what the body depends on, got %q", r.Unpredicted[0].Reason) + } + }) +} diff --git a/mdl/executor/widget_defs_test.go b/mdl/executor/widget_defs_test.go index d055b918bb..90586fbab0 100644 --- a/mdl/executor/widget_defs_test.go +++ b/mdl/executor/widget_defs_test.go @@ -822,6 +822,52 @@ func TestApplyColumnHeaderFallback(t *testing.T) { if h := spec8.Properties[1]; h.PropertyKey != "header" || h.TextTemplate != "colActions" { t.Errorf("Case 8: appended header = %+v, want TextTemplate=colActions", spec8.Properties[1]) } + + // Case 9 (ChipCoV8 #11): an UNNAMED column with no caption and no attribute + // (`column (Caption: '') { … }`, or bare `column { … }`) has nothing to fall + // back to. Its header must still serialize as an empty Forms$ClientTemplate + // — the shape update-widgets writes — not TextTemplate: null, which is + // CE0463. Flagged EmptyTemplate, filled in place. + spec9 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "header", Operation: "texttemplate", TextTemplate: ""}, + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + }, + } + applyColumnHeaderFallback(spec9, "", true) + if len(spec9.Properties) != 2 { + t.Fatalf("Case 9 (unnamed, empty caption): expected 2 properties (no duplicate), got %d", len(spec9.Properties)) + } + if h := spec9.Properties[0]; h.PropertyKey != "header" || h.Operation != "texttemplate" || h.TextTemplate != "" || !h.EmptyTemplate { + t.Errorf("Case 9: header = %+v, want an empty texttemplate with EmptyTemplate=true", h) + } + + // Case 10: unnamed column with NO header prop at all — append an + // EmptyTemplate header. + spec10 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + }, + } + applyColumnHeaderFallback(spec10, "", true) + if len(spec10.Properties) != 2 { + t.Fatalf("Case 10 (unnamed, absent header): expected 2 properties, got %d", len(spec10.Properties)) + } + if h := spec10.Properties[1]; h.PropertyKey != "header" || h.Operation != "texttemplate" || !h.EmptyTemplate { + t.Errorf("Case 10: appended header = %+v, want an empty texttemplate with EmptyTemplate=true", h) + } + + // Case 11: no header slot, unnamed — still untouched (the EmptyTemplate + // header is scoped to datagrid columns, never to header-less items). + spec11 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "staticName", Operation: "texttemplate", TextTemplate: ""}, + }, + } + applyColumnHeaderFallback(spec11, "", false) + if len(spec11.Properties) != 1 || spec11.Properties[0].EmptyTemplate { + t.Errorf("Case 11 (no header slot): must be untouched, got %+v", spec11.Properties) + } } // TestObjectListItemPropertyParamsConvention documents the alias→params diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index f1e152813a..8473e4c102 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -2131,6 +2131,22 @@ func applyColumnHeaderFallback(spec *backend.ObjectListItemSpec, columnName stri fallback = columnName } if fallback == "" { + // An unnamed column with no caption and no attribute has nothing to + // derive a header from. It must still not serialize as TextTemplate: + // null — `mx update-widgets` writes an empty Forms$ClientTemplate for + // the header, so null is CE0463 (ChipCoV8 #11). Flag the header + // EmptyTemplate; scoped by the hasHeaderSlot gate above to datagrid + // columns, never to other texttemplates. + if headerIdx >= 0 { + spec.Properties[headerIdx].Operation = "texttemplate" + spec.Properties[headerIdx].EmptyTemplate = true + return + } + spec.Properties = append(spec.Properties, backend.ObjectListItemProperty{ + PropertyKey: "header", + Operation: "texttemplate", + EmptyTemplate: true, + }) return } if headerIdx >= 0 { diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index e0decc4645..94759d816e 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -764,9 +764,22 @@ visibleValueV3 : IDENTIFIER | QUOTED_IDENTIFIER | keyword ; -// V3 Attribute path: Name, Product/Category, "Order" (quoted to escape reserved words) +// V3 Attribute path: Name, Product/Category, "Order" (quoted to escape reserved words). +// An association hop — every segment before the last — may be module-qualified, +// `Module.Assoc/Attr`, the way XPath and data view sources name it. Without that +// the qualified form missed `Attribute: attributePathV3`, parsed as the division +// `Module.Assoc / Attr` and was refused as an expression (ChipCoV8 #10). The final +// segment stays a bare name: a dotted name with no slash is not a path. attributePathV3 - : (IDENTIFIER | QUOTED_IDENTIFIER | keyword) (SLASH (IDENTIFIER | QUOTED_IDENTIFIER | keyword))* + : (attributePathHopV3 SLASH)* attributePathSegmentV3 + ; + +attributePathHopV3 + : attributePathSegmentV3 (DOT attributePathSegmentV3)? + ; + +attributePathSegmentV3 + : IDENTIFIER | QUOTED_IDENTIFIER | keyword ; // An attribute read from a named object: an enclosing data view, Studio Pro's diff --git a/mdl/linter/report.go b/mdl/linter/report.go index 7cf8cba65e..a51eb28621 100644 --- a/mdl/linter/report.go +++ b/mdl/linter/report.go @@ -55,6 +55,7 @@ var categoryMapping = map[string]string{ "SEC007": "Security", "SEC008": "Security", "SEC009": "Security", + "SEC010": "Security", "CONV006": "Security", "CONV007": "Security", "CONV008": "Security", diff --git a/mdl/linter/rules/page_entity_read_access.go b/mdl/linter/rules/page_entity_read_access.go new file mode 100644 index 0000000000..ec00cf1113 --- /dev/null +++ b/mdl/linter/rules/page_entity_read_access.go @@ -0,0 +1,437 @@ +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "fmt" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/modelsdk/meta" + "github.com/mendixlabs/mxcli/sdk/security" +) + +// PageEntityReadAccessRule (SEC010) reports a data widget whose entity the +// roles that may open its page cannot read. +// +// At Production security the runtime applies entity access to every retrieve +// a page makes, so a list over an entity a role has no read rule for comes out +// EMPTY for that role — and from a microflow data source, the right length with +// every field blank (manage-security skill, measured on 11.14.0). Nothing +// reports it: `check --references`, mxbuild and the runtime log are all clean +// (ChipCoV9: a combo box over System.User filtered by role, empty for every +// non-admin role). +// +// The question is asked per USER role, because that is what a person logs in +// as: a user role that holds one of the page's module roles opens the page, +// and reads the entity if ANY of its module roles has a read rule on it, on +// one of its generalizations (rules are inherited), or on a specialization +// (a list over System.User shows the Administration.Account objects the role +// may read — that is how an Administrator's user list fills). +// +// A specialization's rule fills the rows, but each field only if the rule +// grants read on that member. Members are stored qualified by the entity that +// declares them, so a rule on Administration.Account that reads only +// Administration.Account.FullName and .Email gives a list over System.User its +// rows with System.User.Name blank — every caption empty except the current +// user's (ChipCoV9, measured in the browser on 11.15: ["", "", "demo_manager", +// ""]; the real Administration module's User rule is exactly that). Such a rule +// counts for the specialization, not for the generalization the widget lists. +// A rule with no member rows recorded is taken at its entity-level word. +// System.User's own rule also lets a user role read the users whose roles it +// may manage, so Manage All Roles reads every name (measured on 11.15: an +// Administrator saw all four; a ServiceManager given ManageableRoles +// FieldEngineer saw demo_engineer's name besides their own). +// +// Severity follows what the role sees: +// - no read rule at all → warning, the widget renders empty +// - only XPath-constrained rules → info, it renders the rows the +// constraint allows (often right: "my own requests") +// - System.User, row-scoped → warning, special message: System.User's +// own rule is [id = '[%CurrentUser%]'], so a user picker lists exactly one +// person and, filtered by role, often nobody. +// +// Only at Production. Off checks nothing, and Prototype is stored as +// CheckFormsAndMicroflows: the runtime enforces page and microflow access but +// not entity access, so the data is not withheld there (MPR007 makes the same +// level split, see homePageConsequence). +// +// System entities carry no access rules in the .mpr; their read access comes +// from meta.SystemReadAccess, which is measured from a deployed model.mdp. +type PageEntityReadAccessRule struct{} + +// NewPageEntityReadAccessRule creates the SEC010 rule. +func NewPageEntityReadAccessRule() *PageEntityReadAccessRule { return &PageEntityReadAccessRule{} } + +func (r *PageEntityReadAccessRule) ID() string { return "SEC010" } +func (r *PageEntityReadAccessRule) Name() string { return "PageEntityNotReadable" } +func (r *PageEntityReadAccessRule) Category() string { return "security" } +func (r *PageEntityReadAccessRule) DefaultSeverity() linter.Severity { return linter.SeverityWarning } + +func (r *PageEntityReadAccessRule) Description() string { + return "Checks that the roles allowed on a page may read the entity each of its data widgets lists (at Production security it renders empty for them, silently)" +} + +// RequiredCatalogMode: widgets and permissions are written by REFRESH CATALOG FULL. +func (r *PageEntityReadAccessRule) RequiredCatalogMode() linter.CatalogMode { + return linter.CatalogFull +} + +// readLevel is how much of an entity a set of module roles may read. +type readLevel int + +const ( + readNone readLevel = iota // no read rule + readScoped // only XPath-constrained read rules + readFull // at least one unconstrained read rule +) + +// dataWidgetLabels names the widget types whose entity is a list data source, +// as Studio Pro calls them. A data view is left out: its object usually comes +// in through a page parameter, and mxbuild already reports the attributes in +// it a role cannot read (CE2729). +var dataWidgetLabels = map[string]string{ + "Forms$ListView": "List view", + "Forms$DataGrid": "Data grid", + "Forms$TemplateGrid": "Template grid", + "Forms$ReferenceSelector": "Reference selector", + "Forms$ReferenceSetSelector": "Reference set selector", + "Forms$InputReferenceSetSelector": "Input reference set selector", +} + +// pluggableLabels names the common pluggable data widgets. Any other pluggable +// widget with an entity is reported as "Widget". +var pluggableLabels = map[string]string{ + "com.mendix.widget.web.datagrid.Datagrid": "Data grid", + "com.mendix.widget.web.gallery.Gallery": "Gallery", + "com.mendix.widget.web.combobox.Combobox": "Combo box", + "com.mendix.widget.web.treenode.TreeNode": "Tree node", +} + +func dataWidgetLabel(widgetType string) (string, bool) { + if l, ok := dataWidgetLabels[widgetType]; ok { + return l, true + } + if l, ok := pluggableLabels[widgetType]; ok { + return l, true + } + // A pluggable widget is catalogued under its widget ID, which has no '$'. + if widgetType != "" && !strings.Contains(widgetType, "$") && strings.Contains(widgetType, ".") { + return "Widget", true + } + return "", false +} + +// entityAccessModel is the read rules and inheritance needed to answer +// "may these module roles read entity E". +type entityAccessModel struct { + reads map[string]map[string]readLevel // entity → module role → best level + gen map[string]string // entity → generalization + children map[string][]string // entity → direct specializations + known map[string]bool // entities whose access is known + + // memberOwners records, per entity and module role, the entities that + // declare the members its read rules grant (System.User for + // System.User.Name on an Administration.Account rule). Absent: no member + // rows were recorded for that rule. + memberOwners map[string]map[string]map[string]bool +} + +func (m *entityAccessModel) grant(entity, role string, lvl readLevel) { + if m.reads[entity] == nil { + m.reads[entity] = map[string]readLevel{} + } + if lvl > m.reads[entity][role] { + m.reads[entity][role] = lvl + } +} + +func buildEntityAccessModel(ctx *linter.LintContext) *entityAccessModel { + m := &entityAccessModel{ + reads: map[string]map[string]readLevel{}, + gen: map[string]string{}, + children: map[string][]string{}, + known: map[string]bool{}, + memberOwners: map[string]map[string]map[string]bool{}, + } + // Every entity, platform and marketplace modules included: a list over + // System.User is filled by Administration.Account's rules, so an excluded + // or marketplace module still decides the answer. + if rows, err := ctx.Query(`SELECT QualifiedName, COALESCE(Generalization, '') FROM entities`); err == nil { + for rows.Next() { + var qn, gen string + if rows.Scan(&qn, &gen) != nil { + continue + } + if !strings.HasPrefix(qn, "System.") { + m.known[qn] = true + } + if gen != "" { + m.gen[qn] = gen + } + } + rows.Close() + } + for _, e := range meta.SystemEntities { + qn := "System." + e.Name + if e.Generalization != "" { + m.gen[qn] = e.Generalization + } + grants, ok := meta.SystemReadAccess(qn) + if !ok { + continue + } + m.known[qn] = true + for _, g := range grants { + lvl := readFull + if g.RowScoped { + lvl = readScoped + } + m.grant(qn, g.ModuleRole, lvl) + } + } + for child, parent := range m.gen { + m.children[parent] = append(m.children[parent], child) + } + for p := range ctx.Permissions() { + if p.ElementType == "ENTITY" && p.AccessType == "MEMBER_READ" && p.MemberName != "" { + if i := strings.LastIndex(p.MemberName, "."); i > 0 { + m.addMemberOwner(p.ElementName, p.ModuleRoleName, p.MemberName[:i]) + } + continue + } + if p.ElementType != "ENTITY" || p.AccessType != "READ" || p.MemberName != "" { + continue + } + lvl := readFull + if p.IsConstrained { + lvl = readScoped + } + m.known[p.ElementName] = true + m.grant(p.ElementName, p.ModuleRoleName, lvl) + } + return m +} + +func (m *entityAccessModel) addMemberOwner(entity, role, owner string) { + if m.memberOwners[entity] == nil { + m.memberOwners[entity] = map[string]map[string]bool{} + } + if m.memberOwners[entity][role] == nil { + m.memberOwners[entity][role] = map[string]bool{} + } + m.memberOwners[entity][role][owner] = true +} + +// level returns the best read level any of roles has on entity, counting +// rules on its generalizations and on its specializations. rowsOnly names the +// specializations whose rules the roles may read but that grant none of +// entity's own members (see the type comment): they fill rows with blank +// fields, so they do not raise the level. +func (m *entityAccessModel) level(entity string, roles []string) (best readLevel, rowsOnly []string) { + consider := func(e string) { + for _, r := range roles { + if l := m.reads[e][r]; l > best { + best = l + } + } + } + seen := map[string]bool{} + var chain []string // entity and its generalizations: whose members it shows + for e := entity; e != "" && !seen[e]; e = m.gen[e] { + seen[e] = true + chain = append(chain, e) + consider(e) + } + // grantsShown reports whether role's rule on specialization c grants read + // on a member the widget's entity has. + grantsShown := func(c, r string) bool { + owners, recorded := m.memberOwners[c][r] + if !recorded { + return true + } + for _, e := range chain { + if owners[e] { + return true + } + } + return false + } + var down func(e string) + down = func(e string) { + for _, c := range m.children[e] { + if seen[c] { + continue + } + seen[c] = true + blank := false + for _, r := range roles { + l := m.reads[c][r] + if l == readNone { + continue + } + if !grantsShown(c, r) { + blank = true + continue + } + if l > best { + best = l + } + } + if blank { + rowsOnly = append(rowsOnly, c) + } + down(c) + } + } + down(entity) + return best, rowsOnly +} + +// Check reports each page data widget whose entity some user role that may +// open the page cannot read. +func (r *PageEntityReadAccessRule) Check(ctx *linter.LintContext) []linter.Violation { + reader := ctx.Reader() + if reader == nil { + return nil + } + ps, err := reader.GetProjectSecurity() + if err != nil || ps == nil || ps.SecurityLevel != security.SecurityLevelProduction || len(ps.UserRoles) == 0 { + return nil + } + + pageRoles := map[string][]string{} + for p := range ctx.Permissions() { + if p.ElementType == "PAGE" && p.AccessType == "VIEW" { + pageRoles[p.ElementName] = append(pageRoles[p.ElementName], p.ModuleRoleName) + } + } + if len(pageRoles) == 0 { + return nil + } + + access := buildEntityAccessModel(ctx) + + var violations []linter.Violation + for w := range ctx.Widgets() { + if w.EntityRef == "" || !strings.EqualFold(w.ContainerType, "page") { + continue + } + label, ok := dataWidgetLabel(w.WidgetType) + if !ok || !access.known[w.EntityRef] { + continue + } + roles := pageRoles[w.ContainerQualifiedName] + if len(roles) == 0 { + continue // no role may open it: MPR007 / CE0557 territory + } + var blind, scoped, openers []string + blankVia := map[string]bool{} + for _, ur := range ps.UserRoles { + if !sharesRole(ur.ModuleRoles, roles) { + continue + } + openers = append(openers, ur.Name) + lvl, rowsOnly := access.level(w.EntityRef, ur.ModuleRoles) + if w.EntityRef == "System.User" && ur.ManageAllRoles { + // System.User's own rule also opens the users whose roles this + // user role may manage: with Manage All Roles, every one. + lvl = readFull + } + if lvl == readFull { + continue + } + for _, c := range rowsOnly { + blankVia[c] = true + } + switch lvl { + case readNone: + blind = append(blind, ur.Name) + case readScoped: + scoped = append(scoped, ur.Name) + } + } + if v, ok := r.violation(w, label, roles, openers, blind, scoped, sortedKeys(blankVia)); ok { + violations = append(violations, v) + } + } + return violations +} + +func (r *PageEntityReadAccessRule) violation(w linter.Widget, label string, pageRoles, openers, blind, scoped, blankVia []string) (linter.Violation, bool) { + sort.Strings(pageRoles) + widget := fmt.Sprintf("%s '%s' on page %s lists %s", label, w.Name, w.ContainerQualifiedName, w.EntityRef) + v := linter.Violation{ + RuleID: r.ID(), + Severity: r.DefaultSeverity(), + Location: linter.Location{ + Module: moduleFromQualified(w.ContainerQualifiedName), + DocumentType: "page", + DocumentName: docNameFromQualified(w.ContainerQualifiedName), + }, + } + switch { + case len(blankVia) > 0 && len(blind)+len(scoped) > 0: + // The rows come through a specialization; the fields do not. + who := append(append([]string{}, blind...), scoped...) + sort.Strings(who) + v.Message = fmt.Sprintf("%s, whose rows user role%s %s may read through %s — but that rule grants none of %s's own members, so every field shows blank", + widget, plural(who), quoteList(who), strings.Join(blankVia, ", "), w.EntityRef) + if len(scoped) > 0 && len(blind) == 0 { + v.Message += fmt.Sprintf(" except on the rows %s's own constrained rule allows", w.EntityRef) + if w.EntityRef == "System.User" { + v.Message += " (the current user, and users whose roles that user role may manage)" + } + } + v.Suggestion = fmt.Sprintf("Bind the widget to %s and show a member its rule grants, or grant read on the %s members it displays in that rule", blankVia[0], w.EntityRef) + case len(blind) > 0: + who := fmt.Sprintf("which none of the page's roles (%s) may read", strings.Join(pageRoles, ", ")) + if len(blind) < len(openers) { + who = fmt.Sprintf("which user role%s %s can open the page but may not read", + plural(blind), quoteList(blind)) + } + v.Message = fmt.Sprintf("%s, %s — it renders empty for them (from a microflow data source: every field blank)", widget, who) + v.Suggestion = fmt.Sprintf("GRANT . ON %s (READ *) for a module role those user roles hold, or bind the widget to an entity they may read", w.EntityRef) + if strings.HasPrefix(w.EntityRef, "System.") { + // A module cannot widen access to a System entity (manage-security skill). + v.Suggestion = "A module cannot grant read on a System entity: open the page only to roles holding System.Administrator, or copy the data onto an entity your module owns" + } + case len(scoped) > 0 && w.EntityRef == "System.User": + v.Message = fmt.Sprintf("%s, which lists only the current user for user role%s %s — denormalise the name onto an entity of your own (e.g. Administration.Account) or use a microflow data source that returns that entity", + widget, plural(scoped), quoteList(scoped)) + v.Suggestion = "System.User's own access rule is [id = '[%CurrentUser%]'] for every role; a microflow returning System.User objects delivers the rows with every field blank, so return your own entity" + case len(scoped) > 0: + v.Severity = linter.SeverityInfo + v.Message = fmt.Sprintf("%s, which user role%s %s may read only through an XPath-constrained rule — it renders just the rows the constraint allows", + widget, plural(scoped), quoteList(scoped)) + v.Suggestion = "Intended if the list is meant to be row-scoped; otherwise grant an unconstrained READ to a module role those user roles hold" + default: + return linter.Violation{}, false + } + return v, true +} + +func plural(xs []string) string { + if len(xs) == 1 { + return "" + } + return "s" +} + +func quoteList(xs []string) string { + q := make([]string, len(xs)) + for i, x := range xs { + q[i] = "'" + x + "'" + } + return strings.Join(q, ", ") +} + +func sortedKeys(m map[string]bool) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} diff --git a/mdl/linter/rules/page_entity_read_access_test.go b/mdl/linter/rules/page_entity_read_access_test.go new file mode 100644 index 0000000000..438482b583 --- /dev/null +++ b/mdl/linter/rules/page_entity_read_access_test.go @@ -0,0 +1,339 @@ +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/sdk/security" +) + +// readAccessFixture is a catalog plus project security for SEC010. Pages, +// widgets, entities and rules are added per test. +type readAccessFixture struct { + t *testing.T + cat *catalog.Catalog + sec *security.ProjectSecurity + n int +} + +func newReadAccessFixture(t *testing.T, level string, userRoles ...*security.UserRole) *readAccessFixture { + t.Helper() + cat, err := catalog.NewFromFile(filepath.Join(t.TempDir(), "cat.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { cat.Close() }) + f := &readAccessFixture{t: t, cat: cat, sec: &security.ProjectSecurity{SecurityLevel: level, UserRoles: userRoles}} + for _, mod := range []string{"App", "Administration"} { + f.exec(`INSERT INTO modules_data (Id, Name, ProjectId, SnapshotId) VALUES (?,?,?,?)`, mod+"-id", mod, "default", "s1") + } + return f +} + +func (f *readAccessFixture) exec(q string, args ...any) { + f.t.Helper() + if _, err := f.cat.CatalogDB().Exec(q, args...); err != nil { + f.t.Fatalf("%s: %v", q, err) + } +} + +func (f *readAccessFixture) entity(qn, generalization string) { + f.n++ + mod, name, _ := strings.Cut(qn, ".") + f.exec(`INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, Generalization, ProjectId, SnapshotId) VALUES (?,?,?,?,?,?,?)`, + "e"+string(rune('a'+f.n)), name, qn, mod, generalization, "default", "s1") +} + +func (f *readAccessFixture) read(entity, moduleRole, xpath string) { + f.exec(`INSERT INTO permissions (ModuleRoleName, ElementType, ElementName, AccessType, XPathConstraint, ModuleName) VALUES (?,?,?,?,?,?)`, + moduleRole, "ENTITY", entity, "READ", xpath, moduleFromQualified(entity)) +} + +// memberRead records a MEMBER_READ row as the catalog writes it: the member +// qualified by the entity that DECLARES it (System.User.Name on an +// Administration.Account rule). +func (f *readAccessFixture) memberRead(entity, moduleRole, xpath, member string) { + f.exec(`INSERT INTO permissions (ModuleRoleName, ElementType, ElementName, MemberName, AccessType, XPathConstraint, ModuleName) VALUES (?,?,?,?,?,?,?)`, + moduleRole, "ENTITY", entity, member, "MEMBER_READ", xpath, moduleFromQualified(entity)) +} + +func (f *readAccessFixture) page(page string, moduleRoles ...string) { + for _, r := range moduleRoles { + f.exec(`INSERT INTO permissions (ModuleRoleName, ElementType, ElementName, AccessType, ModuleName) VALUES (?,?,?,?,?)`, + r, "PAGE", page, "VIEW", moduleFromQualified(page)) + } +} + +func (f *readAccessFixture) widget(page, name, widgetType, entity string) { + f.n++ + f.exec(`INSERT INTO widgets_data (Id, Name, WidgetType, ContainerId, ContainerQualifiedName, ContainerType, ModuleName, EntityRef, ProjectId, SnapshotId) + VALUES (?,?,?,?,?,?,?,?,?,?)`, + "w"+string(rune('a'+f.n)), name, widgetType, "c-"+page, page, "PAGE", moduleFromQualified(page), entity, "default", "s1") +} + +func (f *readAccessFixture) check() []linter.Violation { + f.t.Helper() + ctx := linter.NewLintContext(f.cat, navSecurityReader{sec: f.sec}) + return NewPageEntityReadAccessRule().Check(ctx) +} + +const ( + dataGrid = "com.mendix.widget.web.datagrid.Datagrid" + comboBox = "com.mendix.widget.web.combobox.Combobox" +) + +func plainUser() *security.UserRole { + return &security.UserRole{Name: "Member", ModuleRoles: []string{"System.User", "App.User", "Administration.User"}} +} + +func administrator() *security.UserRole { + return &security.UserRole{Name: "Administrator", ModuleRoles: []string{"System.Administrator", "App.Admin", "Administration.Administrator"}} +} + +func oneViolation(t *testing.T, vs []linter.Violation) linter.Violation { + t.Helper() + if len(vs) != 1 { + t.Fatalf("want 1 violation, got %d: %+v", len(vs), vs) + } + return vs[0] +} + +// A page open to App.User with a grid over App.Order, which App.User has no +// read rule on: at Production the grid renders empty for Member. +func TestPageEntityReadAccess_NoReadRuleWarns(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.entity("App.Order", "") + f.read("App.Order", "App.Admin", "") + f.page("App.Orders", "App.User") + f.widget("App.Orders", "dg1", dataGrid, "App.Order") + + v := oneViolation(t, f.check()) + if v.Severity != linter.SeverityWarning { + t.Errorf("severity = %v, want warning", v.Severity) + } + want := "Data grid 'dg1' on page App.Orders lists App.Order, which none of the page's roles (App.User) may read — it renders empty for them" + if !strings.HasPrefix(v.Message, want) { + t.Errorf("message = %q\nwant prefix %q", v.Message, want) + } + if v.RuleID != "SEC010" || v.Location.DocumentName != "Orders" || v.Location.Module != "App" { + t.Errorf("rule/location = %s %+v", v.RuleID, v.Location) + } +} + +// The control: the same page once App.User may read App.Order. +func TestPageEntityReadAccess_ReadRuleIsSilent(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.entity("App.Order", "") + f.read("App.Order", "App.User", "") + f.page("App.Orders", "App.User") + f.widget("App.Orders", "dg1", dataGrid, "App.Order") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// Read granted on a generalization is inherited. +func TestPageEntityReadAccess_GeneralizationRuleCounts(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.entity("App.Party", "") + f.entity("App.Customer", "App.Party") + f.read("App.Party", "App.User", "") + f.page("App.Customers", "App.User") + f.widget("App.Customers", "dg1", dataGrid, "App.Customer") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// A row-scoped rule is often the intent ("my own requests"): info, not warning. +func TestPageEntityReadAccess_RowScopedOnlyIsInfo(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.entity("App.Order", "") + f.read("App.Order", "App.User", "[System.owner = '[%CurrentUser%]']") + f.page("App.Orders", "App.User") + f.widget("App.Orders", "dg1", dataGrid, "App.Order") + + v := oneViolation(t, f.check()) + if v.Severity != linter.SeverityInfo { + t.Errorf("severity = %v, want info", v.Severity) + } + if !strings.Contains(v.Message, "XPath-constrained") { + t.Errorf("message = %q", v.Message) + } +} + +// ChipCoV9: a combo box over System.User for a non-admin role. System.User's +// own rule is [id = '[%CurrentUser%]'] (measured, meta.SystemReadAccess), and +// the role cannot read a specialization in full either. +func TestPageEntityReadAccess_SystemUserForPlainRole(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), administrator()) + f.entity("Administration.Account", "System.User") + f.read("Administration.Account", "Administration.Administrator", "") + f.read("Administration.Account", "Administration.User", "[id = '[%CurrentUser%]']") + f.page("App.Assign", "App.User") + f.widget("App.Assign", "cbEngineer", comboBox, "System.User") + + v := oneViolation(t, f.check()) + if v.Severity != linter.SeverityWarning { + t.Errorf("severity = %v, want warning", v.Severity) + } + for _, want := range []string{ + "Combo box 'cbEngineer' on page App.Assign lists System.User", + "lists only the current user for user role 'Member'", + "denormalise the name onto an entity of your own (e.g. Administration.Account) or use a microflow data source", + } { + if !strings.Contains(v.Message, want) { + t.Errorf("message %q\nlacks %q", v.Message, want) + } + } +} + +// Administrator reads Administration.Account, a specialization of System.User, +// unconstrained — so its user list fills. Nothing to report. +func TestPageEntityReadAccess_SystemUserForAdministrator(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), administrator()) + f.entity("Administration.Account", "System.User") + f.read("Administration.Account", "Administration.Administrator", "") + f.page("App.Assign", "App.Admin") + f.widget("App.Assign", "cbEngineer", comboBox, "System.User") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// ChipCoV9, re-checked: the real Mendix 11.15 Administration module grants +// Administration.User `read (FullName, Email)` on Account UNCONSTRAINED. The +// rows of a list over System.User come through, but System.User's own members +// (Name) are not in that rule, so every caption is blank except the current +// user's (System.User's own [id = '[%CurrentUser%]'] rule). Measured in the +// browser: ["", "", "demo_manager", ""]. The unconstrained entity-level read +// on the specialization made SEC010 call System.User readable and stay silent. +func TestPageEntityReadAccess_SystemUserMembersNotGrantedBySpecialization(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), administrator()) + f.entity("Administration.Account", "System.User") + f.read("Administration.Account", "Administration.User", "") + f.memberRead("Administration.Account", "Administration.User", "", "Administration.Account.FullName") + f.memberRead("Administration.Account", "Administration.User", "", "Administration.Account.Email") + f.page("App.Assign", "App.User") + f.widget("App.Assign", "cbEngineer", comboBox, "System.User") + + v := oneViolation(t, f.check()) + if v.Severity != linter.SeverityWarning { + t.Errorf("severity = %v, want warning", v.Severity) + } + for _, want := range []string{ + "Combo box 'cbEngineer' on page App.Assign lists System.User", + "user role 'Member' may read through Administration.Account", + "Administration.Account", + "blank", + } { + if !strings.Contains(v.Message, want) { + t.Errorf("message %q\nlacks %q", v.Message, want) + } + } +} + +// An Administrator's Account rule grants no System.User member either, yet +// every name shows: System.User's own rule also lets a user role read the users +// whose roles it may manage. Measured on 11.15 (ChipCoV9 scratch): with Manage +// All Roles every name shows; a ServiceManager given ManageableRoles +// (FieldEngineer) saw demo_engineer's name and no other but their own. +func TestPageEntityReadAccess_SystemUserManageAllRolesReadsMembers(t *testing.T) { + admin := administrator() + admin.ManageAllRoles = true + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), admin) + f.entity("Administration.Account", "System.User") + f.read("Administration.Account", "Administration.Administrator", "") + f.memberRead("Administration.Account", "Administration.Administrator", "", "Administration.Account.FullName") + f.page("App.Assign", "App.Admin") + f.widget("App.Assign", "cbEngineer", comboBox, "System.User") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// The control: the same rule, now also granting System.User.Name. +func TestPageEntityReadAccess_SpecializationGrantingInheritedMemberIsSilent(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), administrator()) + f.entity("Administration.Account", "System.User") + f.read("Administration.Account", "Administration.User", "") + f.memberRead("Administration.Account", "Administration.User", "", "Administration.Account.FullName") + f.memberRead("Administration.Account", "Administration.User", "", "System.User.Name") + f.page("App.Assign", "App.User") + f.widget("App.Assign", "cbEngineer", comboBox, "System.User") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// System entities have no rules in the .mpr; without the measured table every +// one of them would be flagged. System.Language is readable by every role. +func TestPageEntityReadAccess_SystemLanguageIsReadable(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.page("App.Profile", "App.User") + f.widget("App.Profile", "cbLanguage", comboBox, "System.Language") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +// An admin-only System entity on a page a plain role may open. +func TestPageEntityReadAccess_AdminOnlySystemEntity(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser(), administrator()) + f.page("App.Sessions", "App.User", "App.Admin") + f.widget("App.Sessions", "dgSessions", dataGrid, "System.Session") + + v := oneViolation(t, f.check()) + if !strings.Contains(v.Message, "which user role 'Member' can open the page but may not read") { + t.Errorf("message = %q", v.Message) + } +} + +// Prototype security is stored as CheckFormsAndMicroflows: entity access is not +// enforced, the data is not withheld, so there is nothing to report. Off too. +func TestPageEntityReadAccess_OnlyAtProduction(t *testing.T) { + for _, level := range []string{security.SecurityLevelPrototype, security.SecurityLevelOff} { + f := newReadAccessFixture(t, level, plainUser()) + f.entity("App.Order", "") + f.page("App.Orders", "App.User") + f.widget("App.Orders", "dg1", dataGrid, "App.Order") + if vs := f.check(); len(vs) != 0 { + t.Errorf("%s: want nothing, got %+v", level, vs) + } + } +} + +// Only list data sources: a data view's object usually arrives as a page +// parameter, and an action button's entity is not a list. +func TestPageEntityReadAccess_IgnoresNonListWidgets(t *testing.T) { + f := newReadAccessFixture(t, security.SecurityLevelProduction, plainUser()) + f.entity("App.Order", "") + f.page("App.Orders", "App.User") + f.widget("App.Orders", "dv1", "Forms$DataView", "App.Order") + f.widget("App.Orders", "btn1", "Forms$ActionButton", "App.Order") + + if vs := f.check(); len(vs) != 0 { + t.Errorf("want nothing, got %+v", vs) + } +} + +func TestPageEntityReadAccess_Identity(t *testing.T) { + r := NewPageEntityReadAccessRule() + if r.ID() != "SEC010" || r.Category() != "security" || r.Name() == "" || r.Description() == "" { + t.Errorf("identity: %s %s %s", r.ID(), r.Category(), r.Name()) + } + if r.RequiredCatalogMode() != linter.CatalogFull { + t.Error("widgets and permissions need the full catalog") + } +} diff --git a/mdl/visitor/visitor_page_qualified_attribute_path_test.go b/mdl/visitor/visitor_page_qualified_attribute_path_test.go new file mode 100644 index 0000000000..06e2ae6487 --- /dev/null +++ b/mdl/visitor/visitor_page_qualified_attribute_path_test.go @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// `Attribute: Module.Assoc/Attr` — an association hop written module-qualified, +// the way XPath and data view sources name it — is an attribute path, exactly +// like the bare `Assoc/Attr`. attributePathV3 accepted only plain identifiers +// between slashes, so the qualified form fell through to the generic +// `keyword: expression` alternative, parsed as the division `M.Assoc / Attr`, +// and check refused it as MDL-WIDGET34 + MDL-WIDGET42 ("cannot hold the +// expression"). +func TestQualifiedAssociationAttributePathParses(t *testing.T) { + cases := []struct { + name, prop, want string + }{ + {"datagrid column", "column c1 (Attribute: FieldService.ServiceRequest_LithoSystem/SerialNumber, Caption: 'S')", + "FieldService.ServiceRequest_LithoSystem/SerialNumber"}, + {"quoted segments", `column c1 (Attribute: "FieldService"."ServiceRequest_LithoSystem"/"SerialNumber")`, + "FieldService.ServiceRequest_LithoSystem/SerialNumber"}, + {"two hops", "column c1 (Attribute: M.A_B/M.B_C/Name)", "M.A_B/M.B_C/Name"}, + {"bare unchanged", "column c1 (Attribute: ServiceRequest_LithoSystem/SerialNumber)", + "ServiceRequest_LithoSystem/SerialNumber"}, + {"plain name unchanged", "column c1 (Attribute: SerialNumber)", "SerialNumber"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create page M.P (Title: 'x', Layout: A.L) { datagrid dg (DataSource: database M.E) { " + tc.prop + " } };" + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + grid := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0] + if len(grid.Children) != 1 { + t.Fatalf("grid children = %d, want 1", len(grid.Children)) + } + got, ok := grid.Children[0].Properties["Attribute"].(string) + if !ok { + t.Fatalf("Attribute = %#v (%T), want the attribute path string %q", + grid.Children[0].Properties["Attribute"], grid.Children[0].Properties["Attribute"], tc.want) + } + if got != tc.want { + t.Errorf("Attribute = %q, want %q", got, tc.want) + } + }) + } +} + +// The qualified hop works on an input widget too, and the final segment stays +// a bare name: `Module.Entity.Attr` with no slash is not an association path. +func TestQualifiedAssociationAttributePathOnTextBox(t *testing.T) { + src := "create page M.P (Title: 'x', Layout: A.L) { dataview dv (DataSource: $P) { textbox tb (Attribute: M.Order_Customer/Name) } };" + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + tb := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0].Children[0] + if got := tb.Properties["Attribute"]; got != "M.Order_Customer/Name" { + t.Errorf("Attribute = %#v, want %q", got, "M.Order_Customer/Name") + } +} diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 42cd924526..9fbe163de3 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -1368,9 +1368,11 @@ func buildAttributePathV3(ctx parser.IAttributePathV3Context) string { text := ctx.GetText() // Strip double quotes or backticks from each path segment if strings.ContainsAny(text, "\"`") { + // A hop may be module-qualified (`"Mod"."Assoc"/Attr`), so each + // segment is unquoted per dotted part, not as one identifier. parts := strings.Split(text, "/") for i, p := range parts { - parts[i] = unquoteIdentifier(p) + parts[i] = unquoteQualifiedName(p) } return strings.Join(parts, "/") } diff --git a/modelsdk/meta/system_access.go b/modelsdk/meta/system_access.go new file mode 100644 index 0000000000..a1d05b49d0 --- /dev/null +++ b/modelsdk/meta/system_access.go @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: Apache-2.0 + +package meta + +// System module read access — who may READ each System entity, as the runtime +// defines it. +// +// The System module's access rules are not in the .mpr: Studio Pro shows them +// read-only and mxbuild writes them into the deployed model. So +// SystemEntities carries no access rules, and anything that reasons about +// "can this role read that entity" had no answer for System at all — a lint +// rule asking it would flag every System entity as unreadable. +// +// MEASURED, not taken from documentation: every row below is read from the +// System domain model in a built `deployment/model/model.mdp` — Mendix +// 11.15.0, security level Production (CheckEverything), an app whose user +// roles each hold exactly one of System.Administrator / System.User. The .mdp +// stores each rule's AllowedUserRoles (user roles, not module roles); a rule +// granted to every user role is recorded here as {System.User, +// System.Administrator}, one granted only to the user roles holding +// System.Administrator (`IsSystemAdministrator` in the .mdp) as +// {System.Administrator}. TestSystemReadAccessMatchesMDP re-checks the table +// against an .mdp given with -mdp. Four System entities the 11.15.0 model has +// and SystemEntities does not (WorkflowSubProcess, WorkflowSubProcessDefinition +// — both System.Administrator only — CdcSnapshotStore, SynchronizationHistory — +// no rule) are left out with them. +// +// Only READ is recorded — a rule counts as granting read when any member of +// it is ReadOnly or ReadWrite. The XPath constraints are abbreviated in the +// comments; RowScoped says only that one exists. + +// System module roles. +const ( + SystemRoleUser = "System.User" + SystemRoleAdministrator = "System.Administrator" +) + +// SystemReadGrant is one System access rule that grants read. +type SystemReadGrant struct { + ModuleRole string // SystemRoleUser or SystemRoleAdministrator + RowScoped bool // the rule carries an XPath constraint +} + +var ( + everyRole = []SystemReadGrant{{SystemRoleUser, false}, {SystemRoleAdministrator, false}} + adminOnly = []SystemReadGrant{{SystemRoleAdministrator, false}} + // Administrator reads every row; System.User only the rows the constraint + // lets through (the current user's own tasks, groups, …). + adminAllUserScoped = []SystemReadGrant{{SystemRoleAdministrator, false}, {SystemRoleUser, true}} + everyRoleScoped = []SystemReadGrant{{SystemRoleUser, true}, {SystemRoleAdministrator, true}} + noRule = []SystemReadGrant{} +) + +// systemReadAccess is keyed by qualified entity name. An entity with an empty +// slice has NO access rule: nobody reads it directly, only through a +// specialization of their own (System.FileDocument, System.Image). +var systemReadAccess = map[string][]SystemReadGrant{ + // [id = '[%CurrentUser%]'] — for EVERY role, Administrator included. Other + // users' ROWS come through a specialization's rules (Administration.Account); + // their System.User MEMBERS only where that rule grants them, or for users + // whose roles the user role may manage. Measured on 11.15.0 (ChipCoV9): a + // list over System.User showed a ServiceManager every row with Name blank + // but their own, and an Administrator (Manage All Roles) every name — the + // Account rules of both grant no System.User member. SEC010 applies this. + "System.User": everyRoleScoped, + // [System.UserRoles = '[%CurrentUser%]' or System.grantableRoles[reversed()]/…/System.UserRoles = '[%CurrentUser%]'] + "System.UserRole": everyRoleScoped, + // [System.owner='[%CurrentUser%]'] + "System.PrivateFileDocument": everyRoleScoped, + + "System.Language": everyRole, + "System.TimeZone": everyRole, + "System.HttpMessage": everyRole, + "System.HttpHeader": everyRole, + "System.HttpRequest": everyRole, + "System.HttpResponse": everyRole, + "System.Paging": everyRole, + "System.ODataResponse": everyRole, + "System.ConsumedODataConfiguration": everyRole, + "System.WorkflowJumpToDetails": everyRole, + "System.WorkflowCurrentActivity": everyRole, + "System.WorkflowActivityDetails": everyRole, + "System.WorkflowRecord": everyRole, + "System.WorkflowActivityRecord": everyRole, + "System.WorkflowEvent": everyRole, + + "System.XASInstance": adminOnly, + "System.Session": adminOnly, + "System.ScheduledEventInformation": adminOnly, + "System.TokenInformation": adminOnly, + "System.SynchronizationError": adminOnly, + "System.SynchronizationErrorFile": adminOnly, + "System.ProcessedQueueTask": adminOnly, + "System.QueuedTask": adminOnly, + "System.TaskQueueToken": adminOnly, + "System.WorkflowDefinition": adminOnly, + "System.WorkflowUserTaskDefinition": adminOnly, + + // [System.WorkflowUserTask_Workflow/System.WorkflowUserTask[State = 'InProgress' and (TargetUsers | TargetGroups/…/User | Assignees = '[%CurrentUser%]')]] + "System.Workflow": adminAllUserScoped, + // [State = 'InProgress' and (TargetUsers | TargetGroups/…/User | Assignees = '[%CurrentUser%]') and Workflow[State != 'Incompatible' and != 'Failed']] + "System.WorkflowUserTask": adminAllUserScoped, + // the same, through System.WorkflowUserTaskOutcome_WorkflowUserTask + "System.WorkflowUserTaskOutcome": adminAllUserScoped, + // [System.WorkflowEndedUserTask_Assignees = '[%CurrentUser%]'] + "System.WorkflowEndedUserTask": adminAllUserScoped, + // the same, through …_WorkflowEndedUserTask + "System.WorkflowEndedUserTaskOutcome": adminAllUserScoped, + // [System.WorkflowGroup_User = '[%CurrentUser%]'] + "System.WorkflowGroup": adminAllUserScoped, + + "System.FileDocument": noRule, + "System.Image": noRule, + "System.Thumbnail": noRule, + "System.Error": noRule, + "System.SoapFault": noRule, + "System.UserReportInfo": noRule, + "System.BackgroundJob": noRule, + "System.AutoCommitEntry": noRule, + "System.UnreferencedFile": noRule, + "System.OfflineCreatedGuids": noRule, + "System.OfflineSynchronizationHistory": noRule, + "System.ChangeHash": noRule, + "System.WorkflowVersion": noRule, + "System.WorkflowActivity": noRule, + "System.WorkflowActivityUserTaskOutcome": noRule, +} + +// SystemReadAccess returns the read-granting access rules of a System entity +// (qualified name) and whether the entity was measured at all. ok == false +// means "unknown", which a caller must not read as "nobody may read it". +func SystemReadAccess(entityQN string) (grants []SystemReadGrant, ok bool) { + grants, ok = systemReadAccess[entityQN] + return grants, ok +} diff --git a/modelsdk/meta/system_access_test.go b/modelsdk/meta/system_access_test.go new file mode 100644 index 0000000000..aeea774ee9 --- /dev/null +++ b/modelsdk/meta/system_access_test.go @@ -0,0 +1,237 @@ +// SPDX-License-Identifier: Apache-2.0 + +// The System module's access rules are not in the .mpr — mxbuild writes them +// into deployment/model/model.mdp. systemReadAccess is a measurement of that +// file; TestSystemReadAccessMatchesMDP re-checks it against a build given with +// -mdp (the same flag the string-length measurement uses): +// +// go test ./modelsdk/meta/ -run SystemReadAccess -mdp /path/to/deployment/model/model.mdp +// +// The build must be at security level Production: below it mxbuild has no +// entity access to write. +package meta + +import ( + "encoding/binary" + "os" + "sort" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" +) + +// Every measured row names an entity SystemEntities knows, so a typo cannot +// make an entity silently "unmeasured". +func TestSystemReadAccessNamesKnownEntities(t *testing.T) { + for qn := range systemReadAccess { + name, ok := strings.CutPrefix(qn, "System.") + if !ok { + t.Errorf("%s: not a System entity", qn) + continue + } + if _, ok := systemEntityByName(name); !ok { + t.Errorf("%s is in systemReadAccess but not in SystemEntities", qn) + } + } +} + +// The rows a page-level lint rule leans on, pinned so an edit to the table is +// a deliberate one. System.User being row-scoped for Administrator too is the +// measured surprise: an Administrator's user list comes from a specialization. +func TestSystemReadAccessKeyRows(t *testing.T) { + cases := []struct { + entity string + want []SystemReadGrant + }{ + {"System.User", []SystemReadGrant{{SystemRoleUser, true}, {SystemRoleAdministrator, true}}}, + {"System.UserRole", []SystemReadGrant{{SystemRoleUser, true}, {SystemRoleAdministrator, true}}}, + {"System.Language", []SystemReadGrant{{SystemRoleUser, false}, {SystemRoleAdministrator, false}}}, + {"System.TimeZone", []SystemReadGrant{{SystemRoleUser, false}, {SystemRoleAdministrator, false}}}, + {"System.Session", []SystemReadGrant{{SystemRoleAdministrator, false}}}, + {"System.WorkflowUserTask", []SystemReadGrant{{SystemRoleAdministrator, false}, {SystemRoleUser, true}}}, + {"System.Workflow", []SystemReadGrant{{SystemRoleAdministrator, false}, {SystemRoleUser, true}}}, + {"System.FileDocument", []SystemReadGrant{}}, + {"System.Image", []SystemReadGrant{}}, + } + for _, c := range cases { + got, ok := SystemReadAccess(c.entity) + if !ok { + t.Errorf("%s: not measured", c.entity) + continue + } + if grantKey(got) != grantKey(c.want) { + t.Errorf("%s: got %s, want %s", c.entity, grantKey(got), grantKey(c.want)) + } + } + if _, ok := SystemReadAccess("System.NoSuchEntity"); ok { + t.Error("an unknown entity must read as unmeasured, not as 'nobody may read it'") + } +} + +func TestSystemReadAccessMatchesMDP(t *testing.T) { + if *mdpPath == "" { + t.Skip("no -mdp given; this test re-measures from a built deployment/model/model.mdp") + } + raw, err := os.ReadFile(*mdpPath) + if err != nil { + t.Fatalf("read %s: %v", *mdpPath, err) + } + measured, err := systemReadAccessFromMDP(raw) + if err != nil { + t.Fatal(err) + } + if len(measured) == 0 { + t.Fatalf("%s carries no System entity access — is it a Production-level deploy build?", *mdpPath) + } + for qn, got := range measured { + want, ok := systemReadAccess[qn] + if !ok { + t.Logf("%s: measured %s, not in the table (add it if pages bind it)", qn, grantKey(got)) + continue + } + if grantKey(got) != grantKey(want) { + t.Errorf("%s: table says %s, %s says %s", qn, grantKey(want), *mdpPath, grantKey(got)) + } + } + for qn := range systemReadAccess { + if _, ok := measured[qn]; !ok { + t.Errorf("%s is in the table but not in %s", qn, *mdpPath) + } + } +} + +func grantKey(gs []SystemReadGrant) string { + parts := make([]string, 0, len(gs)) + for _, g := range gs { + s := g.ModuleRole + if g.RowScoped { + s += "[xpath]" + } + parts = append(parts, s) + } + sort.Strings(parts) + return "{" + strings.Join(parts, ", ") + "}" +} + +// systemReadAccessFromMDP maps each System entity in a deployed model to its +// read-granting rules, translating the stored user roles to System module +// roles: a rule for every user role is {System.User, System.Administrator}, +// one for exactly the IsSystemAdministrator user roles is +// {System.Administrator}, and one for exactly the others is {System.User}. +func systemReadAccessFromMDP(raw []byte) (map[string][]SystemReadGrant, error) { + var docs []bson.M + for off := 0; off+4 <= len(raw); { + size := int(binary.LittleEndian.Uint32(raw[off : off+4])) + if size < 5 || off+size > len(raw) { + break + } + var doc bson.M + if err := bson.Unmarshal(raw[off:off+size], &doc); err == nil { + docs = append(docs, doc) + } + off += size + } + + admins, users := map[string]bool{}, map[string]bool{} + var walk func(v any) + walk = func(v any) { + switch x := v.(type) { + case bson.M: + if x["$Type"] == "Security$UserRole" { + name, _ := x["Name"].(string) + if isAdmin, _ := x["IsSystemAdministrator"].(bool); isAdmin { + admins[name] = true + } else { + users[name] = true + } + } + for _, val := range x { + walk(val) + } + case bson.A: + for _, val := range x { + walk(val) + } + } + } + for _, d := range docs { + walk(d) + } + + sameSet := func(got []string, want map[string]bool) bool { + if len(got) != len(want) { + return false + } + for _, g := range got { + if !want[g] { + return false + } + } + return true + } + all := map[string]bool{} + for r := range admins { + all[r] = true + } + for r := range users { + all[r] = true + } + + out := map[string][]SystemReadGrant{} + for _, d := range docs { + if d["$Type"] != "DomainModels$DomainModel" { + continue + } + entities, _ := d["Entities"].(bson.A) + for _, ev := range entities { + e, _ := ev.(bson.M) + qn, _ := e["QualifiedName"].(string) + if !strings.HasPrefix(qn, "System.") { + continue + } + grants := []SystemReadGrant{} + rules, _ := e["AccessRules"].(bson.A) + for _, rv := range rules { + rule, _ := rv.(bson.M) + if rule == nil || !ruleGrantsRead(rule) { + continue + } + var roles []string + if ar, ok := rule["AllowedUserRoles"].(bson.A); ok { + for _, r := range ar { + if s, ok := r.(string); ok { + roles = append(roles, s) + } + } + } + xpath, _ := rule["XPathConstraint"].(string) + scoped := strings.TrimSpace(xpath) != "" + switch { + case sameSet(roles, all): + grants = append(grants, SystemReadGrant{SystemRoleUser, scoped}, SystemReadGrant{SystemRoleAdministrator, scoped}) + case sameSet(roles, admins): + grants = append(grants, SystemReadGrant{SystemRoleAdministrator, scoped}) + case sameSet(roles, users): + grants = append(grants, SystemReadGrant{SystemRoleUser, scoped}) + default: + grants = append(grants, SystemReadGrant{"?" + strings.Join(roles, "+"), scoped}) + } + } + out[qn] = grants + } + } + return out, nil +} + +func ruleGrantsRead(rule bson.M) bool { + members, _ := rule["MemberAccesses"].(bson.A) + for _, mv := range members { + m, _ := mv.(bson.M) + switch m["AccessRights"] { + case "ReadOnly", "ReadWrite": + return true + } + } + return false +} diff --git a/mods/companion/hooks/register.tsx b/mods/companion/hooks/register.tsx index d1d6cb6684..a04c44b1ba 100644 --- a/mods/companion/hooks/register.tsx +++ b/mods/companion/hooks/register.tsx @@ -12,6 +12,7 @@ import type { Project, Source, TreeNode } from '../types' import { assumeDescribable, changesModel, + diagramView, firstLine, nodeId, pageOf, @@ -35,6 +36,9 @@ const sourceTop = atom({ plugin: 'mxcli-companion', key: 'sourceTop' } as const, // Whether the band above the prompt offers the Project pane: once per project, // until the person opens the pane or says not now. const welcome = atom({ plugin: 'mxcli-companion', key: 'welcome' } as const, false) +// How a diagram is shown, and whether the tree gives up its column to it. +const diagram = atom({ plugin: 'mxcli-companion', key: 'diagram' } as const, { zoom: 'actual', x: 0 } as { zoom: 'fit' | 'actual'; x: number }) +const treeHidden = atom({ plugin: 'mxcli-companion', key: 'treeHidden' } as const, false) // project-tree reads the whole model; on a large app that is the slow call. const TREE_TIMEOUT_MS = 120_000 @@ -44,6 +48,9 @@ const DESCRIBE_TIMEOUT_MS = 60_000 const SIDE_BY_SIDE_MIN_COLUMNS = 90 // Rows the pane's own chrome takes (header, the gap, a pager line per column). const CHROME_ROWS = 4 +// CSS pixels per cell on the surfaces that draw Svg (a monospace cell at the +// desktop's font size); used to crop a diagram to the column at actual size. +const CELL_PX = 8 // A burst of agent writes reloads once, this long after the last one. const RELOAD_DEBOUNCE_MS = 1_500 @@ -167,7 +174,40 @@ const scheduleReload = async ($: Engine) => { if (mine === reloadGeneration) await load($, current.mpr) } -const press = async ($: Engine, id: string) => { +// Surfaces whose element table has Svg; the terminal draws no pictures. +const DRAWS_SVG = new Set(['desktop', 'vscode', 'mobile']) +// The most an Svg element takes (its `source` cap). +const SVG_MAX_CHARS = 131_072 + +// drawDomainModel asks mxcli for the module's domain model as an SVG and shows +// it; the tree's own list of the entities stays beside it. +const drawDomainModel = async ($: Engine, node: TreeNode) => { + const current = await read($, project) + if (current.state !== 'loaded' || !node.qualifiedName) return + const qn = node.qualifiedName + const children = (node.children ?? []).map(c => c.label) + const key = `svg:${qn}` + const cached = described.get(key) + if (cached !== undefined) { + await update($, source, (): Source => ({ state: 'diagram', label: node.label, qn, svg: cached, children })) + return + } + await update($, diagram, d => ({ ...d, x: 0 })) + await update($, source, (): Source => ({ state: 'drawing', label: node.label, qn })) + const out = await run($, ['describe', '-p', current.mpr, '--format', 'svg', 'domainmodel', qn], DESCRIBE_TIMEOUT_MS) + let next: Source + if (out.exitCode !== 0) { + next = { state: 'failed', label: node.label, qn, error: firstLine(out.stderr || out.stdout) } + } else if (out.stdout.length > SVG_MAX_CHARS) { + next = { state: 'failed', label: node.label, qn, error: `the diagram is too large to draw here (${out.stdout.length} characters)` } + } else { + described.set(key, out.stdout) + next = { state: 'diagram', label: node.label, qn, svg: out.stdout, children } + } + await update($, source, (s): Source => (s.state === 'drawing' && s.qn === qn ? next : s)) +} + +const press = async ($: Engine, id: string, surface: string) => { const current = await read($, project) if (current.state !== 'loaded') return const node = findNode(current.tree, id) @@ -179,6 +219,8 @@ const press = async ($: Engine, id: string) => { await update($, selected, () => id) if (node.describable) { await describeSelected($, node) + } else if (node.type === 'domainmodel' && DRAWS_SVG.has(surface)) { + await drawDomainModel($, node) } else { await update($, source, (): Source => ({ state: 'container', @@ -277,6 +319,9 @@ export const register: Register = (on, options) => { const sel = await read($, selected) const src = await read($, source) const sideBySide = e.props.bodyColumns >= SIDE_BY_SIDE_MIN_COLUMNS + const hideTree = sideBySide && src.state === 'diagram' && (await read($, treeHidden)) + const treeColumns = Math.min(56, Math.max(28, Math.floor(e.props.bodyColumns * 0.38))) + const sourceColumns = sideBySide && !hideTree ? e.props.bodyColumns - treeColumns - 2 : e.props.bodyColumns // Side by side, each column is one screen tall and pages on its own; stacked, // the pane scrolls as usual and the columns draw whole. const pageRows = sideBySide ? Math.max(8, e.props.scroll.bodyRows - CHROME_ROWS) : Number.MAX_SAFE_INTEGER @@ -338,7 +383,7 @@ export const register: Register = (on, options) => { return ( {sel === id ? '›' : ' '}{' '.repeat(depth)} -