diff --git a/.claude/commands/mxcli-dev/review.md b/.claude/commands/mxcli-dev/review.md index 53edca9946..596868287a 100644 --- a/.claude/commands/mxcli-dev/review.md +++ b/.claude/commands/mxcli-dev/review.md @@ -11,7 +11,8 @@ burned us before. ## Steps 1. Run `gh pr view` and `gh pr diff` (or `git diff main...HEAD`) to read the change. -2. Work through the CLAUDE.md "PR / Commit Review Checklist" in full. +2. Work through CLAUDE.md's "Working Rules for a Change" (the evidence bar) and + the subsystem checklists at the end of this file, in full. 3. Then check every row in the Recurring Findings table below — flag any match. 4. Report: blockers first, then moderate issues, then minor. Include a concrete fix option for every blocker (not just "this is wrong"). @@ -68,3 +69,86 @@ proactively. Add a row after every review that surfaces something new. - [ ] Recurring Findings table updated with any new pattern. - [ ] If docs-only PR: every function name, path, and PR reference verified against live code before approving. + +## The subsystem checklists + +These moved out of CLAUDE.md, where they were re-read into every session but only +apply when a change touches that subsystem. The evidence bar for a bug fix, and +the one-thing-per-commit rule, stay there because they govern how the work is +done rather than how it is reviewed. + +### Overlap & duplication +- [ ] Check `docs/11-proposals/` for existing proposals covering the same functionality +- [ ] Search the codebase for existing implementations (grep for key function names, command names, types) +- [ ] Check `mdl-examples/doctype-tests/` for existing test coverage of the feature area +- [ ] Verify the PR doesn't re-document already-shipped features as new + +### Syntax design for MDL features +New or modified MDL syntax must follow the design guidelines. See [ADR-0003: MDL is SQL-shaped](docs/13-decisions/0003-mdl-is-sql-shaped.md) for the underlying decision and rejected alternatives; the design checklist below operationalises it. +- [ ] **Design skill consulted** — read `.claude/skills/design-mdl-syntax.md` before designing syntax +- [ ] **Follows standard patterns** — uses `create`/`alter`/`drop`/`show`/`describe`, not custom verbs +- [ ] **Reads as English** — a business analyst understands the statement on first reading +- [ ] **Qualified names** — uses `Module.Element` everywhere, no implicit module context +- [ ] **Property format** — uses `( key: value, ... )` with colon separators, one per line +- [ ] **LLM-friendly** — one example is sufficient for an LLM to generate correct variants +- [ ] **Diff-friendly** — adding one property is a one-line diff + +### Version compatibility +New features that depend on a specific Mendix version must be version-gated: +- [ ] **Registry entry** — feature added to `sdk/versions/mendix-{9,10,11}.yaml` with correct `min_version` +- [ ] **Executor pre-check** — `checkFeature()` called before BSON writes, with actionable error and hint +- [ ] **Test coverage** — version-gated tests use `-- @version:` directives or `requireMinVersion()` +- [ ] **Skill updated** — `.claude/skills/version-awareness.md` updated if the feature has a workaround for older versions + +### Backend abstraction compliance +All executor code must go through the backend abstraction layer. **`sdk/mpr` no longer exists** — the package was deleted once its importer count reached zero, so reaching past the abstraction is now a compile error rather than a rule to remember. See [ADR-0002: Backend Abstraction Layer](docs/13-decisions/0002-backend-abstraction.md) for the context and alternatives. The codec (`modelsdk`) engine is the only local engine — the legacy `sdk/mpr` backend was deleted (`docs/plans/2026-09-14-retire-legacy-engine.md`), and `--engine`/`MXCLI_ENGINE` survive only as a warning-only no-op. It routes **all** document types — domain models included — through the codec, not a codec/legacy hybrid; see [ADR-0004: Full codec engine](docs/13-decisions/0004-full-codec-engine.md). Where the codec path cannot yet reproduce a construct, the backend **refuses** the op rather than dropping data. The backend interface speaks the **semantic model**, not gen/BSON or AST types — gen+codec are the MPR backend's internal storage adapter, one of several (MPR, MCP/PED, a future storage format); see [ADR-0005](docs/13-decisions/0005-semantic-model-interface-currency.md). CREATE is model→gen; fidelity-sensitive ALTER uses backend-internal gen-mutation, not a model round-trip. +- [ ] **No engine internals in the executor** — executor files must not reach into `modelsdk/mpr`, `modelsdk/codec` or `modelsdk/gen` directly; use `ctx.Backend.*` instead. A method missing from the backend gets implemented there, not bypassed +- [ ] **New backend methods on the interface** — any new data access or mutation goes in the appropriate interface in `mdl/backend/` (e.g., `DomainModelBackend`, `MicroflowBackend`), not as a direct SDK call +- [ ] **MPR implementation in `mdl/backend/mpr/`** — the concrete implementation lives here; all BSON/reader/writer logic stays in this package +- [ ] **Mock stub in `mdl/backend/mock/`** — every new backend method has a `Func`-field stub with a descriptive `"MockBackend.X not configured"` error default (not `nil, nil`) +- [ ] **Compile-time interface check** — new backend implementations have `var _ backend.SomeInterface = (*impl)(nil)` +- [ ] **ALTER operations use mutator pattern** — page/workflow mutations go through `ctx.Backend.OpenPageForMutation()` / `OpenWorkflowForMutation()`, not inline BSON construction +- [ ] **New shared types in `mdl/types/`** — a type used by more than one layer goes in `mdl/types/` and the others alias it (`type Foo = types.Foo`), never as duplicate definitions. A same-shape duplicate compiles and tests green; it shows up only as an assignment failure *across* the boundary, naming the same type on both sides of "want". `modelsdk/mpr/version.ProjectVersion` was that case and is now an alias — the guard is a compile-time assertion (`var _ *types.ProjectVersion = (*version.ProjectVersion)(nil)`, `version_alias_test.go`), which builds only under an alias and so is stronger than anything a test body can assert +- [ ] **Map iteration is deterministic** — any map iterated for serialization output must sort keys first (`sort.Strings(keys)` pattern); non-deterministic output causes flaky diffs and BSON instability +- [ ] **Pluggable widgets via WidgetEngine** — new pluggable widget support uses `.def.json` + `WidgetRegistry`; no hardcoded BSON widget builders in the executor + +### Full-stack consistency for MDL features +New MDL commands or language features must be wired through the full pipeline: +- [ ] **Grammar** — rule added to `MDLParser.g4` (and `MDLLexer.g4` if new tokens) +- [ ] **Parser regenerated** — `make grammar` run; generated files in `mdl/grammar/parser/` are **not** committed (they are regenerated by `make` at build time) +- [ ] **AST** — node type added in `mdl/ast/` +- [ ] **Visitor** — ANTLR listener bridges parse tree to AST in `mdl/visitor/` +- [ ] **Executor** — thin handler in `mdl/executor/` dispatches to `ctx.Backend.*`; no BSON in the handler +- [ ] **Backend method** — data access or mutation wired through `mdl/backend/` interface and implemented in `mdl/backend/mpr/` +- [ ] **LSP** — if the feature adds formatting, diagnostics, or navigation targets, wire it into `cmd/mxcli/lsp.go` and register the capability +- [ ] **DESCRIBE roundtrip** — if the feature creates artifacts, `describe` should output re-executable MDL +- [ ] **VS Code extension** — if new LSP capabilities are added, update `vscode-mdl/package.json` + +### Test coverage +- [ ] New packages have test files +- [ ] New executor commands have MDL examples in `mdl-examples/doctype-tests/` +- [ ] **MDL syntax changes** — any PR that adds or modifies MDL syntax must include working examples in `mdl-examples/doctype-tests/` +- [ ] **Bug fixes** — every bug fix should include an MDL test script in `mdl-examples/bug-tests/` that reproduces the issue, so the fix can be verified in Studio Pro if applicable. **Three numbering namespaces meet in that directory**: the historical files are named after `mendixlabs/mxcli` **PR** numbers (`261-mx9-microflow-roundtrip.mdl` is upstream PR #261), issues filed on the fork are `ako/mxcli` numbers — and the two sequences already collide on 261–266 — while a few names are a **Mendix version** with the dot dropped (`1113-database-query-type-enum.mdl` is Mendix 11.13, not issue 1113). Name a file after a fork issue with a topic prefix (`mapping-261-object-handling-backup.mdl`) and write the reference qualified (`ako/mxcli#261`) wherever it appears, or the number silently resolves to the wrong thing +- [ ] Integration paths (not just helpers) are tested +- [ ] Tests don't rely on `time.Sleep` for synchronization — use channels or polling with timeout + +### Security & robustness +- [ ] Unix sockets use restrictive permissions (`os.Chmod(path, 0600)`) +- [ ] File I/O is not in hot paths (event loops, per-keystroke handlers) — cache in memory +- [ ] No silent side effects on typos (e.g., auto-creating resources on misspelled names should be flagged) +- [ ] Method receivers are correct (pointer vs value) for mutations + +### Documentation +- [ ] **Skills** — new features documented in `.claude/skills/` (syntax, examples, gotchas) +- [ ] **CLI help (Cobra)** — `mxcli` subcommand help text updated (Cobra `Short`/`Long`/`Example` fields) +- [ ] **CLI help (syntax topics)** — `cmd/mxcli/syntax/features_*.go` updated with new/changed MDL syntax; new `SyntaxFeature` entries added for new document types; `OR MODIFY` / `OR REPLACE` variants reflected in existing `Syntax` fields; accessible via `mxcli syntax ` and REPL `help` +- [ ] **Syntax reference** — `docs/01-project/MDL_QUICK_REFERENCE.md` updated with new statement syntax +- [ ] **MDL examples** — working examples added to `mdl-examples/` for new commands +- [ ] **Site docs** — `docs-site/src/` pages added or updated for user-facing features + +### Code quality +- [ ] Refactors are applied consistently across all relevant files (grep for the old pattern) +- [ ] Manually maintained lists (keyword lists, type mappings) are flagged as maintenance risks +- [ ] Design docs match the actual implementation — remove or update stale plans +- [ ] Numeric type conversions are bounds-checked — `float64→int` casts need overflow guards (`±2^53` for safe integer range); silent overflow produces garbage in serialized output +- [ ] `convert.go` updated when structs in `mdl/types/` gain or lose fields — `TestFieldCountDrift` will catch this at test time, but `convert.go` must be updated before merging diff --git a/.claude/skills/diagnose-ce0463.md b/.claude/skills/diagnose-ce0463.md index b8ae6e48b6..ff7b2e84c8 100644 --- a/.claude/skills/diagnose-ce0463.md +++ b/.claude/skills/diagnose-ce0463.md @@ -196,3 +196,18 @@ Ordered by how often they have actually been the answer. - **Test any candidate fix against the bundled package too.** Pruning the fields the `update-widgets` reference omits fixes 2 widgets on Data Widgets 3.10 and takes the bundled 3.4 from **0 → 139**. + +## Pluggable Widget Templates + +For pluggable widgets (DataGrid2, ComboBox, Gallery, etc.), templates must include **both** `type` AND `object` fields: +- `type`: Widget PropertyTypes schema (defines what properties exist) +- `object`: Default WidgetObject with all property values + +**CE0463 "widget definition changed" error**: This error occurs when the Object's property structure doesn't match the Type's PropertyTypes. Always extract templates from Studio Pro-created widgets, not programmatically generated ones. See `sdk/widgets/templates/README.md` for details. For debugging CE0463 and other BSON issues, follow the workflow in `.claude/skills/debug-bson.md`. + +## `mxcli fix widgets` clears CE0463 after a headless install + +`fix widgets` / `fix design-properties` run `mx update-widgets` and +`mx rename-design-properties` and **persist** the result without their MPR v2 -> v1 +collapse: let the tool convert, read the units back, restore v2, write the changed +ones through mxcli's writer. Measured 203 -> 0 errors on a vanilla 11.12.1 app. diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 5859918bf2..97b9f90e87 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -118,3 +118,6 @@ {"area": "cmd/mxcli", "date": "2026-09-18", "symptom": "mendixlabs/mxcli#1025: `mxcli syntax` advertises `mxcli syntax workflow user-task targeting` in its own help and answers `Unknown topic: workflow user-task targeting`. Same for `workflow user-task` and `workflow parallel-split`, all of which `mxcli syntax workflow` lists as sub-topics; `--json` was the only route that reached them.", "cause": "The CLI built its path with `strings.Join(args, \".\")` and never split an argument, so a topic handed over as ONE string — a quoted copy-paste, a tool wrapper, `sh -c` — became the path `workflow user-task targeting`, which matches nothing. The REPL's `help` had resolved multi-word topics since it was written (`resolveHelpPath`, greedy hyphen-joining): one question, two answers, and the CLI held the weaker copy. The #955 segment-match fallback could not save it either — it passed the DOTTED path to `BySegmentMatch`, and no segment contains a '.', so that fallback was silently dead for every multi-word query.", "file": "cmd/mxcli/syntax/topic.go (new: Lookup, topicWords, resolvePath), cmd/mxcli/help.go, mdl/executor/cmd_misc.go (resolveHelpPath deleted), mdl/grammar/domains/MDLSettings.g4 (helpStatement, helpTopicWord), mdl/visitor/visitor_query.go (ExitHelpStatement); tests cmd/mxcli/cmd_syntax_test.go, cmd/mxcli/syntax/topic_test.go, mdl/executor/cmd_misc_test.go, mdl/visitor/visitor_help_topic_test.go; example mdl-examples/bug-tests/syntax-1025-topic-drilldown.mdl", "insight": "**The spaces in the reported error message were the whole diagnosis, and reading them as a paraphrase cost an hour.** The command prints the path it built, and the CLI joins on '.', so `Unknown topic: workflow user-task targeting` cannot come from the command as documented — it can only come from the topic arriving as a single argument. Every line of the report follows from that and nothing else does: `syntax workflow` works (one word), `--json` works (the flag is not part of the topic), the three multi-word forms fail. Take a quoted error message literally, character for character, before assuming the reporter retyped it. **The reported version is downloadable and settles it in one run**: `mxcli setup mxcli`'s own URL shape (`releases/download//mxcli-linux-amd64`, NOT the goreleaser `_Linux_x86_64.tar.gz` that 404s) fetched v0.20.0, where the unquoted command works and the quoted one reproduces the message verbatim — so 'fixed since' and 'never broken' were both wrong. **The guard that matters is not the three cases from the report** but `TestEveryRegisteredPathIsReachableBySpelling`: every registered path, tried dotted, as separate arguments, and as one string. The registry prints dotted paths and then tells the reader to drill down with words, so a spelling that does not resolve is the command contradicting its own output; a per-case test would have passed the day someone added a topic with a new shape. Control: stub the whitespace split in `topicWords` and it fails with the reported path, spaces and all. **The grammar half has a trap the CLI half does not, and only the EXISTING suite caught it.** `helpStatement: IDENTIFIER (identifierOrKeyword)*` is the grammar's catch-all — a statement that is just an identifier and some words — so whatever it can swallow, it swallows from the statement that should have had it. Widening it to `(DOT? helpTopicWord)*` to take `help workflow.user-task` made `Sec.ApiUser` a complete statement of its own, and `create module role Sec.ApiUser` then parsed, WITH NO PARSE ERROR, as CREATE MODULE (named \"role\") followed by a help topic — two statements, wrong types, six unrelated security tests red. `(helpTopicWord (DOT? helpTopicWord)*)?` — a topic word before any dot — leaves `.ApiUser` unconsumable and restores the old disambiguation. Bisect a grammar regression by SHAPE, not by reading the ATN: adding the unused rule alone was clean, the hyphen alone was clean, the leading optional DOT was the whole of it, and three regenerations said so in about a minute. **When widening a permissive rule, the test to add is not for the new spelling but for what the rule must still NOT swallow** (TestHelpRuleDoesNotSwallowATrailingQualifiedName).", "refs": ["mendixlabs/mxcli#1025", "#955"]} {"area": "cmd/mxcli", "date": "2026-09-18", "symptom": "`mxcli report` scores a project against rules the team disabled in `lint-config.yaml`. `mxcli lint` honours the config, the report's SCORE does not move, so the score cannot be calibrated at all. Reported at 66/100 against a 99/100 blank-app baseline, where 61 of 86 findings were two deliberately-accepted rules", "cause": "`cmd_report.go` never called `linter.FindConfigFile`/`LoadConfig` — it went straight from `linter.New` to `BuildReport`. Separately it carried its own INLINE copy of the built-in rule list, one rule behind `builtinLintRules()` (missing MDL-FLOW01), so the two commands scored one project against two rule sets. One root cause: report re-implemented lint's setup instead of sharing it", "file": "`cmd/mxcli/cmd_report.go`, `cmd/mxcli/cmd_lint.go`, new `cmd/mxcli/lint_setup.go` (`projectLintRules`, `applyLintConfig`), `mdl/linter/linter.go` (`RuleEnabled`)", "insight": "Same class as #904 in the opposite direction: there a silently reduced rule set made the score falsely HIGH, here an unread config makes it falsely LOW — and both are invisible because a score carries no provenance. **A value test cannot guard the inline copy**: both commands build rules inside a cobra RunE, so nothing a unit test can call notices a second list being re-added. The guard is therefore structural — grep `cmd_report.go` for `lint.AddRule(rules.New` — with a POSITIVE CONTROL first (assert `builtinLintRules` still constructs rules) so it cannot pass vacuously, the same shape as `scripts/check-tunnel-deps.sh`. Take the LintContext out of `applyLintConfig`'s signature: `NewLintContext(nil, nil)` panics, and a nil-guard added only to make a test compile is how a helper acquires behaviour nothing needs", "refs": ["#525", "#904"]} {"area": "cmd/mxcli/marketplace", "date": "2026-09-20", "symptom": "`mxcli marketplace install ... -p app.mpr` (project named by a RELATIVE path) fails with `install the package's bundled files: package entry \"manifest.json\" would write outside the project` \u2014 after the module has already been transplanted into the model. `SHOW MODULES` lists the module, `mx check` is clean, but no bundled file (themesource/, widgets/) landed and the command exited 1. Absolute `-p` paths work.", "cause": "`InstallPackageFiles` builds `dst := filepath.Join(projectDir, clean)` and then checks `strings.HasPrefix(filepath.Clean(dst), filepath.Clean(projectDir)+os.PathSeparator)`. With projectDir == \".\" (from `filepath.Dir(\"app.mpr\")`), Join drops the dot, so dst is `manifest.json` and the prefix is `./` \u2014 every legitimate entry fails the zip-slip guard. The guard was checking the joined path (the shape CodeQL recognises) but never anchored the project directory first.", "file": "`cmd/mxcli/marketplace/update.go` (`InstallPackageFiles`: `filepath.Abs(projectDir)` before the loop), test `cmd/mxcli/marketplace/install_relative_dir_test.go`", "insight": "A containment guard has two inputs and both must be canonical \u2014 the entry AND the root. The traversal tests only ever passed an absolute t.TempDir(), so the root was canonical by accident and the relative case had no coverage; the first real CLI invocation with `-p app.mpr` hit it. Worse, the transplant runs BEFORE the file step, so the failure lands on a half-installed module: the model has it, the disk does not, and the exit code says failure. Order the steps so the cheap, reversible file copy can be validated before the model write, or at least say in the error that the model was already changed. Prove-by-revert done: the new test fails on the unpatched function with the exact reported message.", "refs": []} +{"area": "cmd/mxcli/docker", "date": "2026-09-22", "symptom": "`windows-process-regression` fails intermittently on TestKillProcessGroup_ReapsGrandchildAndUnblocksWait: `cmd.Wait() did not return after killProcessGroup`, ~20.5s (the select deadline), and the GitHub runner then logs `Terminate orphan process: pid (NNNN) (PING)`. killProcessGroup reports no error. Reruns pass, so it reads as 'Windows CI is flaky' and gets attributed to whatever PR happened to be red.", "cause": "The test's readiness marker named the wrong process. The `spawn` helper mode started `cmd /c ping -n 60 127.0.0.1` and wrote `grandchild-started` immediately after `gc.Start()` returned — but Start() only guarantees `cmd.exe` was CREATED; `ping.exe`, which is what ends up holding the inherited stdout pipe, does not exist yet. The test raced ahead to killProcessGroup, `taskkill /F /T` enumerated a tree `ping.exe` had not joined, returned 0, and ping survived holding the write end, so cmd.Wait() never saw EOF.", "file": "`cmd/mxcli/docker/procgroup_windows_test.go` (helper gains a `grandchild` mode that announces ITSELF, with its pid, over the inherited pipe; the test then asserts processAlive on that pid before killing), parser split to `cmd/mxcli/docker/procgroup_marker_test.go` + TestGrandchildPID", "insight": "A readiness marker is only worth what it proves about the process the test is ABOUT. Emitting it from the parent after Start() proves the parent reached a line of code, which is the one thing never in doubt. Emit it from the process under test, over the channel under test — the unix half already did exactly this (`sh -c 'sleep 60 & echo $!; wait'` plus kill(gpid,0)) and the Windows half had silently diverged, so the fix was porting the sibling's handshake rather than inventing one. Two second-order traps: a deadline bump cannot fix this (the grandchild is never killed, so no amount of waiting helps) and would have buried it; and the marker parser must reject a line not yet terminated by \\n, since a truncated pid parses as a plausible different pid. The same-commit control that settled blame: sha 0710968d ran the identical workflow twice, `push` (35715042749) green and `pull_request` (35715076433) red — when a job is suspected flaky, look for two runs of one commit before reading the diff.", "refs": ["ako/mxcli#594", "ako/mxcli#597", "ako/mxcli#601"]} +{"area": "cmd/mxcli/diag", "date": "2026-09-22", "symptom": "`mxcli diag loop-report --json` reports `\"failed\": 0` across a real 442-invocation log while runs were genuinely exiting non-zero. Read next to `\"unclosed\": 10` in the same object it says 'nothing failed', which is the opposite of the truth. The text report never printed the field at all, so the misreading was reachable only through --json, where no surrounding prose corrects it.", "cause": "The field counted a different population than its name claimed. `rep.Failed++` fires only for an invocation that CLOSED (wrote session_end) whose summary carried errors_count > 0, and errors_count is diaglog's STATEMENT-level counter \u2014 so the only path reaching it is a run that kept going after a failed statement, i.e. `exec --continue-on-error`. A run that actually fails exits through os.Exit, which skips the deferred Close() and PersistentPostRun, writes no session_end, and lands in `unclosed`. The two populations are disjoint by construction and `failed` is near-always zero. Nothing was broken; the name was.", "file": "`cmd/mxcli/diag_loop_report.go` (`loopReport.Failed` -> `StatementErrors`, json tag `failed` -> `runs_with_statement_errors`; renderLoopReport prints it only when non-zero and takes io.Writer so the text output is testable), tests `cmd/mxcli/diag_loop_report_test.go` (TestStatementErrorsIsDisjointFromUnclosed, TestJSONKeyNamesWhatItMeasures, TestStatementErrorLineIsPrintedOnlyWhenNonZero)", "insight": "A metric that is always zero fails silently in the one direction nobody checks: it is indistinguishable from good news, so it is never investigated. This one survived review and a whole feature PR because every local test run genuinely had no --continue-on-error invocations, so 0 was CORRECT in the test set and wrong in the field \u2014 only a 442-invocation log from a real project surfaced it. Two rules fell out. (1) Assert the JSON key literally, not the Go field: the key is the interface the wrong conclusion was drawn through, and a Go-side rename leaves the tag behind. (2) Never print a counter's zero beside a related non-zero counter; suppress it, or the pair reads as a comparison. The larger fix \u2014 making `failed` mean failed \u2014 needs an exit-code path through ~250 os.Exit sites in cmd/mxcli, since Go has no atexit; `unclosed` already carries that signal and the report explains it. Prove-by-revert done both ways: restoring the tag fails the key test with `\"failed\":2` in the payload, relaxing the guard to >= 0 prints `Finished with failed statements: 0` directly under `Did not close: 2`, which is the reported symptom exactly.", "refs": ["ako/mxcli#617", "ako/mxcli#620"]} +{"area": "cmd/mxcli/check", "date": "2026-09-22", "symptom": "`make check-mdl` Error 1 in CI right after the #618 empty-script guard landed. The failing fixture, mdl-examples/doctype-tests/15-fragment-examples.test.mdl, got: 'produced no statements, but it is not empty. The parser could not begin reading it. First line that did not parse: create module FragTest;' \u2014 for a 416-line file that parses perfectly well (18 statements) when copied to a plain .mdl name. The filename was the whole difference.", "cause": "Two defects stacked. (1) MINE: cmd_check.go renders a .test.mdl through testrunner.CheckSource \u2014 a test block is a microflow BODY, so check parses the RENDERING, not the file \u2014 but the #618 guard was given `string(content)`, the file as read. A file with no @test block renders to nothing, so the guard saw zero statements against non-empty ORIGINAL text and quoted a source line the parser had never been handed. (2) PRE-EXISTING: that fixture declares no @test at all. It is a syntax demo misnamed .test.mdl (its sibling 15b-fragment-slots-examples.mdl is plain), so CheckSource rendered it to nothing, check printed 'Check passed!' on zero statements, and `make check-mdl` had been reporting PASS over 416 lines nothing ever read \u2014 concealing a real MDL-PAGE20 violation (page param $Customer, url with no {Customer} segment).", "file": "`cmd/mxcli/cmd_check.go` (guard takes `source`, the parsed text, not `content`; empty rendering from non-empty content gets its own branch), `cmd/mxcli/empty_script.go` (`noTestsDeclaredError`), fixture renamed to `mdl-examples/doctype-tests/15-fragment-examples.mdl` with the url fixed, tests `cmd/mxcli/empty_script_test.go`", "insight": "A guard that reports on text OTHER than what the parser consumed will eventually quote a line the parser never saw, and it reads as authoritative precisely because it names a line. Wherever a command transforms its input before parsing \u2014 a renderer, a preprocessor, a macro pass \u2014 every diagnostic downstream must be fed the transformed text, or it describes a file that was never compiled. The wider lesson is about what the guard FOUND: #618 is 'a silent no-op is the worst outcome', and the repo's own check-mdl suite contained an instance \u2014 a file passing because nothing read it. A suite that reports PASS per file cannot distinguish 'checked and clean' from 'not checked'; the tell was available all along in the statement count, which was 0. When a new guard fails CI, check whether it found a second instance of its own bug before assuming it is a false positive: here it was BOTH, and only fixing the diagnosis would have left the misnamed fixture green and unread. Prove-by-revert done end-to-end: restoring `string(content)` reproduces the CI message verbatim on the same bytes under the .test.mdl name.", "refs": ["ako/mxcli#618", "ako/mxcli#619", "ako/mxcli#1103"]} diff --git a/.claude/skills/fix-issue/findings/mdl-backend.jsonl b/.claude/skills/fix-issue/findings/mdl-backend.jsonl index e081d80995..a6322046e2 100644 --- a/.claude/skills/fix-issue/findings/mdl-backend.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-backend.jsonl @@ -123,3 +123,5 @@ {"area": "mdl/backend", "date": "2026-09-20", "symptom": "`DROP ENTITY` left every CROSS-MODULE association pointing at the deleted entity in place. Dropping the local BY-ID (FROM) end made mxbuild 11.14.0 unable to LOAD the project: `System.AggregateException \u2026 (The given key '' was not present in the dictionary.)` at `StreamingBsonUnitReader.ResolvePostponedProperties()` \u2014 no CE code, no document named, so the obvious reading is 'the project is corrupt, restore from git'. Dropping the BY-NAME (TO) end is milder and still wrong: CE1613 at the cross-module association. `show associations` shows a raw GUID where the parent entity should be.", "cause": "`removeAssocsReferencing` swept `dm.AssociationsItems()` and asserted `*genDm.Association` per item, so the SEPARATE `CrossAssociations` collection was never looked at. Fixed with `removeCrossAssocsReferencing`, matching BOTH ends because a cross-module association addresses them differently \u2014 FROM by element id (local), TO by qualified name (another module) \u2014 called in DeleteEntity locally and in its cascade over the other domain models.", "file": "`mdl/backend/modelsdk/domainmodel_alter.go` (removeCrossAssocsReferencing, DeleteEntity)", "insight": "**Reported against a view entity; nothing about it was view-entity specific.** The reporter met it dropping view entities (whose associations are DERIVED from OQL, so there is no CREATE ASSOCIATION to undo) and filed it that way. The first probe \u2014 a view entity and its source entity in the SAME module \u2014 did not reproduce at all, and that negative is the useful one: it says the variable is cross-module, not view-ness. A plain `create association A.X from A.X to B.Y` plus `drop entity A.X` reproduces the identical crash. Two lessons: when a repro fails, vary the dimension the report did not mention before doubting the report, and treat a collection-typed `.(*T)` assertion in a cascade as a place where a sibling type hides. mxbuild's diagnostic distinguishes the two ends for free \u2014 a dangling 16-byte pointer is a LOAD crash, a dangling qualified name is CE1613 \u2014 so testing only one end proves half the fix.", "refs": ["#553", "#556"]} {"area": "mdl/backend", "date": "2026-09-21", "symptom": "`alter settings workflows add group 'Auditors'` reports \"Added workflow group: Auditors (3 group(s))\" and writes nothing \u2014 `show workflow groups` still lists 2, and `mx check` is 0 errors either way", "cause": "`UpdateProjectSettings` overlays the workflows part field by field onto the PRESERVED raw part, so a child LIST that nothing rebuilds is carried through from disk unchanged. Adding `Groups` to the semantic model and to the read path is not enough; the write needs `settingsoverlay.WorkflowGroups(ws, rawPart)`. Identical shape to the enabled-language list the same function already documents", "file": "`mdl/backend/modelsdk/settings_write.go` + `mdl/settingsoverlay/settingsoverlay.go` (`WorkflowGroups`)", "insight": "For anything under Settings$ProjectSettings, the executor's success message proves NOTHING \u2014 it reports the in-memory model, and the overlay is where a list quietly fails to land. Assert on the re-read document, not the handler's output. Two more things a reference project settles in one dump and a guess gets wrong: the `Groups` typed-array marker is 2, not the 3 every other settings child list uses (`ArrayMarker` preserves a stored one, but the fallback matters on a fresh list), and the element's `$ID` is the RUNTIME's identity \u2014 a booted 11.13.0 app keys `system$workflowgroup.modelguid` on it, byte-identical once the .NET GUID field order is undone, so re-minting it on a description edit would orphan every group membership with a perfectly valid model. Control: deleting the one overlay call reproduces the symptom verbatim. mendixlabs/mxcli#272", "refs": ["mendixlabs/mxcli#272"]} {"area": "mdl/backend", "date": "2026-09-21", "symptom": "`CREATE OR MODIFY VIEW ENTITY` that changed ONLY the OQL printed `Unchanged view entity: \u2026` while `describe entity` showed the new query stored. Changing the attribute list as well reported `Modified` correctly, which is why it hid. Also: the OQL document's unit was replaced under a FRESH GUID on every run, even a byte-identical one, so an MDL-generated project could never come back clean in git (one of the four units #556 measured).", "cause": "A view entity's OQL lives in a separate `DomainModels$ViewEntitySourceDocument` unit, and the executor DELETED it and INSERTED a fresh one on every write. `ReportMutation` downgrades the verb when writes were offered and none landed, but the counters are incremented only at the update choke points (`writer_core.go` reconcileWithStored / MoveUnit) \u2014 `InsertUnit` is not counted at all. So the domain-model unit was offered and correctly elided, the OQL write was invisible, and the report believed the half it could see. Fixed with `WriteViewEntitySourceDocument`, which keeps the stored unit's id and goes through `UpdateRawUnit` \u2192 reconcile: an identical query is elided, a changed one lands and is counted, duplicates are still cleared.", "file": "`mdl/backend/modelsdk/move_view_write.go` (WriteViewEntitySourceDocument, encodeViewEntitySourceDocument), `mdl/executor/cmd_entities.go`", "insight": "**The first fix that comes to mind \u2014 count InsertUnit \u2014 would have swapped a false \"Unchanged\" for a false \"Modified\".** Measuring before changing is what caught it: re-running a BYTE-IDENTICAL script still re-minted the source document's unit id, so counting inserts would have made every view-entity statement report Modified forever. The right fix was the one ADR-0008 already mandates (wire the write path to canon.Reconcile), and it fixes the churn and the verb together. Generalisation worth remembering: any content that reaches storage through `InsertUnit` is invisible to the elision check, so a statement whose only landing write is a NEW unit can still be mis-reported \u2014 `MoveUnit` has a comment explaining it was counted for exactly this reason, and insert/delete were missed. Control the fix on the identical re-run, not just the changed one.", "refs": ["#583", "#556", "#910"]} +{"area":"mdl/backend","date":"2026-09-22","symptom":"modelsdk/mpr/version.ProjectVersion declared its own struct with the same seven fields as mdl/types.ProjectVersion instead of aliasing it, so a *version.ProjectVersion could not be passed where a *types.ProjectVersion was wanted and vice versa — two unrelated Go types that both print as 'ProjectVersion'.","cause":"The deleted sdk/mpr/version aliased the canonical type (`type ProjectVersion = types.ProjectVersion`); this copy declared a duplicate. CLAUDE.md's shared-types rule asks for the alias, and nothing enforced it. The duplication survived the legacy-engine retirement because it compiles perfectly — the two declarations are field-for-field identical, so only an assignment ACROSS the boundary reveals them as different types.","file":"modelsdk/mpr/version/version.go","fix":"Made it an alias. The four methods it redeclared (IsAtLeast, IsAtLeastFull, String, IsMPRv2) were verified semantically identical to types' first — IsAtLeast differed only in early-return style, same truth table — and now come from types. IsSupported/SupportsFeature could not survive as methods on an aliased type and had ZERO callers anywhere (measured), so they went with Feature, MinVersion, featureVersions and SupportedVersionRange; that map called itself 'the fallback when the YAML registry is unavailable' and the live registry is sdk/versions/mendix-{9,10,11}.yaml via checkFeature.","insight":"A same-shape duplicate type is invisible to every signal except an assignment across the package boundary: it compiles, tests pass, and the error it eventually produces names the same type on both sides of 'want'. So the guard is a COMPILE-TIME assertion, not a runtime test — `var _ *types.ProjectVersion = (*version.ProjectVersion)(nil)` builds only under an alias and fails to build under a duplicate, which is strictly stronger than anything a test body can assert. Write it before the fix and watch it fail to compile; that failure IS the reproduction. Two measurements that made the cleanup safe rather than brave: diff the method BODIES before assuming the redeclarations are redundant (identical behaviour, different style, is the common case and the dangerous one is the near-miss), and count callers of anything the alias forces you to drop — here six exported symbols had zero. Unrelated trap hit while verifying: four cmd/mxcli tests that read skill files failed once in a full `go test ./...` interleaved with `make check-mdl`, which runs sync-skills (rsync --delete into cmd/mxcli/skills/). They pass in isolation, on clean main, and in an uninterleaved full run — do not attribute a skills-reading test failure to your change without re-running it alone."} +{"area": "mdl/backend", "date": "2026-09-22", "symptom": "`create workflow … overview page X` reports `Created workflow` and exit 0 and stores NOTHING — the written unit carries no page reference and not even the page's qualified name as a string. `mx check` passes (a workflow with no overview page is valid) and `describe workflow` omits the clause, so nothing reveals the loss. Running `alter workflow … set overview page X` afterwards DOES write it, which is what makes the split visible", "cause": "Two fields for one concept, never joined: the executor set semantic `Workflow.OverviewPage` (`cmd_workflows_write.go:170`) and `workflowToGen` only ever read `Workflow.AdminPage`, which nothing set. The READ half was wrong in the mirror direction — `workflowFromGen` took `g.OverviewPageQualifiedName()`, so even the correctly-written ALTER read back empty and the catalog's overview-page reference edge never fired", "file": "`sdk/workflows/workflow.go` (the two fields collapsed to one), `mdl/backend/modelsdk/workflow_write.go` (`workflowToGen`), `mdl/backend/modelsdk/workflow_read.go` (`workflowOverviewPageName`)", "insight": "**The Model SDK's StructureVersionInfo settles which of two rival property names is real, in one grep**: `npm pack mendixmodelsdk` then `src/gen/workflows.js` gives `overviewPage: {deleted: \"9.11.0\"}` and `adminPage: {introduced: \"9.11.0\"}` — so AdminPage (a `Workflows$PageReference` CHILD, not a by-name string) is the stored property, and `generated/metamodel` agrees by declaring AdminPage and no OverviewPage. `modelsdk/gen` declares BOTH, which is how a reader and a writer ended up on opposite sides of a 9.11 rename inside one package. **The version branch CLAUDE.md's overlay rule would demand is dead here, and that is a measurement not an assumption**: `workflowToGen` writes `WorkflowV2`, introduced in 11.1.0, unconditionally — so no reachable project wants the pre-9.11 key. Write one spelling, READ both (a read fallback invents nothing). **The differential that proves it on a real build**: same script, same project, only the write suppressed — control 0 errors, fixed `CE7410 \"The selected page 'Overview' should accept a parameter of type 'Workflow'\"` on mxbuild 11.6.6. mxbuild can only validate a page it can see, so the error IS the evidence; with a valid overview page both variants are 0 errors, which is the usual weak-signal trap. Useful side-finding: an overview page takes **System.Workflow**, while a user task's page takes **System.WorkflowUserTask** — two pages, two parameters. NOT fixed: no check rule for CE7410 yet, and `WorkflowV2` being written unconditionally is questionable for a 10.x project. Same shape as the `create … comment 'text'` bug (findings/mdl-grammar.jsonl 2026-08-25): grep for `stmt.X = …` / `wf.X = …` with no matching read. Tests `mdl/backend/modelsdk/workflow_overview_page_test.go`; repro `mdl-examples/bug-tests/workflow-586b-overview-page-dropped.mdl`", "refs": ["ako/mxcli#586"], "ce": ["CE7410"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 3a491f5698..f340793d9f 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -670,3 +670,7 @@ {"area": "mdl/executor", "date": "2026-09-21", "symptom": "`create or modify entity` drops an attribute a LATER script added, silently. Reported shape: entity created in 01-domain-core.mdl, a calculated attribute added in 03-logic.mdl (its microflow does not exist until then); re-running slice 01 ALONE rebuilt the entity from its own statement and removed the attribute, with `Modified entity: ServiceCore.LithoSystem` as the only output. It surfaced two slices later as `[CE1613] \"The selected attribute 'ServiceCore.LithoSystem.OpenRequestCount' no longer exists.\" at Text 'dtOpen'` — an error naming the PAGE, never the script that removed the attribute. `mxcli check … -p app.mpr --references` said \"Check passed!\".", "ce": "CE1613", "rules": ["MDL087"], "cause": "Half the ask was already shipped and half was not, and the report could not tell them apart. exec's warning (droppedEntityMembers, findings #24, landed 320a304 two weeks before the report) DOES fire — measured on a real 11.6.6 project re-running the reporter's slice 01, it prints the attribute by name — so the reporter was on an older binary. What genuinely did not exist was the issue's second ask: `check` had no project-aware pass for member loss at all, so the one command that runs BEFORE anything is written was the silent one. Added CheckEntityMemberDrops (MDL087, warning) to cmd_check.go's catalog-backed tier, and refactored droppedEntityMembers to share its comparison.", "file": "`mdl/executor/validate_entity_member_drops.go` (new: entityMemberSet, droppedMembers, CheckEntityMemberDrops), `mdl/executor/cmd_entities.go` (droppedEntityMembers now delegates), `cmd/mxcli/cmd_check.go` (projectViolations)", "insight": "**Reproduce before theorising when the report predates a fix in the same area** — exec already printed the exact line the issue asks for, so reading the issue text alone leads either to 'already fixed, close it' or to reimplementing the shipped half. Running the reporter's own sequence against a real project separated the two halves in one command each, and the isolated-slice check printing `Check passed!` is what identified the actual gap. **A check-time twin of an exec-time warning must NOT be the same computation.** exec is per-statement because it is applying statements; check sees the whole script, so it has to be the NET effect — a script that rebuilds an entity and then `alter entity … add attribute`s the members back loses nothing, and that is the IDIOMATIC full-script order, so a per-statement port would warn on every correct script and be switched off within a day. **Intent has to be tracked, not inferred from the outcome**: `drop attribute` / `rename attribute` / `drop entity` produce the same before/after diff as the accident, and a pure diff cannot separate them. Both of those are separate controls, and the naive implementation fails each one specifically (measured: stubbing the net/intent logic fails TestMDL087_ExplicitRemovalIsSilent on 3 of 4 spellings while the positive test still passes — so the positive test alone proves nothing). **One comparison, two layers**: the audit system fields and an omitted `extends` were reported by exec and would have been missed by a second hand-written diff, which is why droppedEntityMembers was refactored onto the shared entityMemberSet rather than copied. An audit pseudo-type (`AutoOwner`) is a FLAG, not an attribute — exec `continue`s past it — so counting it as one makes a faithful restatement read as a drop.", "refs": ["ako/mxcli#562", "findings #24", "findings #13"]} {"area": "mdl/executor", "date": "2026-09-21", "symptom": "`retrieve $AccountList from Administration.Account sort by System.Language.Code asc;` — MDL that `mxcli describe` had just emitted — passed `mxcli check` and was refused by `mxcli exec`: \"sort by attribute 'System.Language.Code' does not belong to entity 'Administration.Account'\". Reported as a check/exec inconsistency (mendixlabs/mxcli#1152); the real defect is that the round trip cannot replay its own output for any sort over an association reached from an ANCESTOR.", "cause": "inferSortEntityRefSteps searched ONE domain model — the retrieved entity's own module — for associations whose parent was the retrieved entity ITSELF, and qualified the association it found with the retrieved entity's module. All three assumptions hold only when the hop starts on the retrieved entity in its own module. Administration.Account reaches System.Language through System.User_Language, declared on System.User and stored in the System module: parent is an ancestor, the domain model is another module's, and the qualified name carries THAT module. Rewritten as a generalization-chain walk that looks each ancestor up in its own module and qualifies the association with the module storing it; the destination end is matched with entityIsSubtypeOf rather than by equality, since an association may point at a specialization of the entity that declares the attribute.", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (inferSortEntityRefSteps); tests `mdl/executor/cmd_microflows_sort_association_test.go`, `mdl/backend/modelsdk/microflow_retrievesort_test.go`; example `mdl-examples/bug-tests/microflow-1152-sort-over-association.mdl`", "insight": "**The second control is the one that pays.** Reverting the fix reproduces the refusal, which only proves the test fires. The control that taught something was building a binary that DERIVES the hop and does not WRITE it — exec succeeds and mxbuild 11.12.3 answers CE7247 \"Cannot sort on attribute 'System.Language.Code'. Attribute 'System.Language.Code' is not an attribute of entity 'Administration.Account'\" — the executor's refusal message almost word for word, from the other end of the pipeline. That is what fixes the qualified name as load-bearing: the stored EntityRefStep must read System.User_Language, and the pre-existing code would have written Administration.User_Language had it found anything at all. **Skip the theory that check is missing a rule**: check has no sort-attribute rule at all and resolves no hops, so it was never going to disagree with exec here — the inconsistency in the report is a symptom of the false refusal, not a second defect. **Known residue, stated because the round trip rests on it**: DESCRIBE emits only the attribute's qualified name, so where several associations reach one entity the replay picks the nearest ancestor's first and can silently land on the other hop. Spelling the hop needs grammar (sortColumn is qualifiedName|IDENTIFIER, no `/` path) and is a language change, not a fix."} {"area": "mdl/executor", "date": "2026-09-21", "symptom": "Follow-up to the sort-hop inference fix: with the hop derivable but not SAYABLE, `describe → exec` still silently changed the program wherever two associations reach the same entity. Measured on 11.12.3 with Order_ShipTo and Order_BillTo (both Order -> Address): a microflow sorting by the BILLING address came back sorting by the SHIPPING one, `mx check` 0 errors on both sides. Same for a page datasource's sort bar.", "cause": "DESCRIBE emitted only the sort attribute's qualified name and the reader never looked at the hop at all — `sortItemsFromRaw` read AttributeRef.Attribute and skipped AttributeRef.EntityRef, so the association was written and never read back. MDL had no spelling for it either (`sortColumn : (qualifiedName | IDENTIFIER)`). Closed end to end: sortColumn takes `qualifiedName (SLASH qualifiedName)*` (the shape MDLCatalog.g4 already uses for Association/Entity), SortColumnDef/OrderByItemV3 carry the hops, the executor resolves the NAMED association instead of inferring, both readers reconstruct EntityRef.Steps, both describers emit `Assoc/.../Attr`, and the page writers moved from attributeRefToGen to inputAttributeRefToGen. Inference stays as the fallback, so every script written before still works.", "file": "`mdl/grammar/domains/MDLPage.g4` (sortColumn) + `mdl/ast/ast_page.go`/`ast_page_v3.go` + `mdl/visitor/visitor_microflow_statements.go` (sortColumnHops) + `visitor_page_v3.go` + `mdl/executor/cmd_microflows_builder_actions.go` (resolveSortAssociationPath, lookupSortHop, entityChainModules) + `cmd_microflows_format_action.go` + `cmd_pages_builder_v3.go` (resolveAssociationAttributePathForEntity) + `cmd_pages_describe_datasource.go` (sortAttributeHops, sortColumnPath) + `mdl/backend/modelsdk/microflow_read_actions.go` (entityRefStepsFromRaw) + `widget_write.go` + `sdk/pages/pages_datasources.go` (GridSort.AttributeRefSteps)", "insight": "**The measurement that decides whether a lossy describer is worth a language change is a CONSTRUCTED one.** The corpus agrees with the inference rule by construction — every document mxcli itself wrote stores the association inference would have picked, so the round trip is a fixed point on everything to hand and looks faithful. The case that matters had to be built: two associations to one entity, then the stored hop edited to the one inference does NOT pick. Byte-patching the .mxunit is enough and takes a minute — `Order_ShipTo` and `Order_BillTo` are the same length, so a `sed` on the BSON needs no resize — and the replay flipped it back immediately. **Control on a binary that drops the hop, not just on one that reverts the fix**: reverting only proves the test fires, while dropping the hop gets mxbuild to say CE7247 \"Cannot sort on attribute … is not an attribute of entity …\" — the executor's own refusal message from the other end of the pipeline, which is what proves the EntityRef load-bearing rather than cosmetic. **Two reads were missing, not one**: the microflow reader and the page reader each drop the hop separately, and fixing only the half named in the report would have shipped a describer that emits the path for microflows and silently drops it for pages. **The strongest round-trip evidence is 'Unchanged'** — with identity preservation and write elision, replaying DESCRIBE output on a correct implementation elides the write entirely, so `Unchanged microflow: …` is a stronger result than any byte comparison."} +{"area": "mdl/executor", "date": "2026-09-22", "symptom": "`CREATE OR REPLACE LAYOUT` re-run with an identical statement reported `Replaced layout …` and dirtied 3 files EVERY time. Measured on a blank 11.14.0 project, three runs produced three different .mxunit filenames (8aa37ee1… -> fd6e9c96… -> 6d2a…): the unit was deleted and re-inserted under a fresh GUID, so git shows a delete plus an untracked add rather than a modified file. Second, unreported symptom found by the control: a layout MOVEd into a folder was filed back into the module root on every rewrite (`show layouts` Folder column Layouts -> empty).", "cause": "execCreateLayout collected the stored layout's id into `toDelete`, deleted it, and called CreateLayout with a freshly built layout — CreateLayout goes through InsertUnit under a newly minted id, and InsertUnit is not a canon.Reconcile choke point. The #556 net (carryIdentityFromRemovedUnit) cannot cover it: that keys on the unit ID and this path re-mints it, so there is nothing to reconcile the re-insert against. The folder half has the same single cause: there is no FOLDER clause on CREATE LAYOUT, so buildLayoutV3 always sets ContainerID to the module root, and only an INSERT applies that to the unit's row.", "file": "`mdl/executor/cmd_pages_layout_v3.go` (execCreateLayout), `mdl/backend/modelsdk/layout_write.go` (UpdateLayout), `mdl/backend/page.go` + `mdl/backend/mock/`", "insight": "**When a `create or modify` handler churns, look for delete+create before looking at the codec.** This is the third instance in one week — REST client (#556), view entity OQL document (#583), layout (#600) — and all three were the same shape and took the same fix: rewrite the stored unit through UpdateRawUnit instead of replacing it. The tell is cheap: `ls` the .mxunit filenames across two runs. A CHANGED filename means delete+insert (fix the handler); a same filename with different bytes means the codec or a carry (fix canon). **A storage-layer net that keys on the unit ID cannot cover a path that re-mints the ID** — worth stating because #556's fix reads like it generalised, and it does not reach here. **The folder defect is the one the tests would not have found**: it only appears once a layout has been moved, which no unit test set up and no reported symptom mentioned; it surfaced from running the faulted binary through a MOVE, which is why the control is worth running on more than the reported case. **Do not trust the issue's severity**: #556 ties this to #553 (project unloadable). Measured with a real page bound to the churned layout, mxbuild reports 0 errors on both variants, because pages resolve layouts by qualified name and not by unit GUID — so `mx check` is not a control for this class at all and the version-control diff is the only signal.", "refs": ["ako/mxcli#600", "ako/mxcli#556", "ako/mxcli#583", "ako/mxcli#932", "mendixlabs/mxcli#1063"]} +{"area":"mdl/executor","date":"2026-09-22","symptom":"`UPDATE WIDGETS` prints a per-property `Warning: Failed to set …` for every assignment and then reports `Updated 2 widget(s)`, plus `Note: Run 'refresh catalog full force' to update the catalog with changes`, and exits 0. `describe styling` afterwards shows nothing was written","cause":"`updated++` sat OUTSIDE the assignment loop and was unconditional, so the counter meant \"this widget was found\" and was reported as \"Updated\". The same counter gated `mutator.Save()`, so a container whose every assignment failed was still saved","file":"`mdl/executor/cmd_widgets.go` (`updateOutcome`, `updateWidgetsInContainer`, `execUpdateWidgets` summary)","insight":"**A success counter incremented in the wrong loop is invisible to every test that only checks the happy path** — the failures were already being printed correctly one line above the lie. Split the outcome into the three things that actually happen (changed / matched-but-unwritable / in-catalog-but-not-in-document) rather than adding a boolean: rounding the third into either of the others is how a stale catalog reads as success. **Bound the severity before writing it up**: the rebuilt document was semantically identical, so ADR-0008 elision skipped the write — measured, no `mprcontents/` unit changed mtime and `mx check` stayed at 0 errors, making this a reporting defect and not a data one. Worth saying, because \"claims success after failing\" otherwise reads as corruption. **The DRY RUN had the same defect one step earlier and is the worse half**, since the syntax help tells you to run it first: it printed `Would set …` without attempting anything. Fixed by running the assignments against `pagemutator.Probe()` — the discardable copy `mxcli check` already uses for ALTER PAGE SET — so the preview reports `Cannot set`. Reuse that seam rather than re-deriving what a setter accepts; a preview that re-implements the rule drifts from it in exactly the direction that hurts","refs":["ako/mxcli#520","ako/mxcli#515"]} +{"area":"mdl/executor","date":"2026-09-22","symptom":"`alter page … set '' = on ` dead-ended — `set` reaches first-class properties and the stored widget's PLUGGABLE property bag, and a design property lives in `Appearance.DesignProperties`. The only spelling that worked was `alter styling`, a second statement for the same operation","cause":"No resolution from a STORED widget to its theme-registry key, so `set` could not tell a design property from a mistyped pluggable one and had to assume the latter","file":"`mdl/backend/pagemutator/probe.go` (`WidgetStorageType`); `mdl/executor/design_property_routing.go` (new); `cmd_alter_page.go` (`applySetPropertyMutator`); `mdl/backend/pagemutator/mutator.go` (the now-stale error message)","insight":"**The resolver the routing needed already existed with zero callers.** `bsonTypeToDesignPropsKey` ($Type → theme key) had never been referenced, so it had never been validated against anything; ako/mxcli#509 deliberately avoided standing up a third consumer of the concept before something needed it, and this was that something. **Do not assert the two key maps are consistent — they are not, and both directions have measured reasons.** $Type-only: `DataGrid`/`Gallery` are the NATIVE widgets, which the MDL keywords no longer produce (`datagrid`→Data grid 2's id via pluggableKeywordIDs), so the stored path resolves MORE than the inline one. Keyword-only: `header`/`footer` map to \"Header\"/\"Footer\" but MDL builds BOTH as `Forms$DivContainer`, and Atlas declares no such groups — so the inline design-property validation for a header widget misses and skips the widget silently, the same shape pluggableKeywordIDs records for combobox/gallery/image. A test that pins both exclusive SETS with their reasons is the useful shape; a consistency assertion fails on correct code. **Route only on a positive theme declaration for THIS widget's type** — routing on \"the theme says nothing, so it must be a design property\" turns a typo into a silently-written design property. **Prove the two statements are the same operation on bytes, not on reasoning**: write via `alter styling`, then run the `alter page` form and count rewritten units — 0 means elision found them semantically equal. `Altered page` is ALTER PAGE's fixed verb and is NOT the elision verb, so it proves nothing. Knock-on: the #1135 error message named `alter styling` as the route, which became stale the moment `set` learned the route — and a test asserted that wording, so it had to be inverted like the others","refs":["ako/mxcli#515","ako/mxcli#509","ako/mxcli#511","mendixlabs/mxcli#1135"]} +{"area":"mdl/executor","date":"2026-09-22","symptom":"No way to set a design property across pages — \"every data grid compact and striped\" was one statement per page, and the bulk command that looked right (`update widgets`) writes only the pluggable property bag","cause":"ALTER PAGE's design-property SET (the singular half of ako/mxcli#515) had no plural sibling; MDL's only bulk page statement was `ALTER PAGES … SET LAYOUT`","file":"`mdl/grammar/MDLParser.g4` (`alterPagesStylingStatement`); `mdl/ast/ast_alter_page.go`; `mdl/visitor/visitor_alter_page.go`; `mdl/executor/cmd_alter_pages_styling.go` (new)","insight":"**The selector is the whole design problem, and a name cannot be it**: a widget name is unique only within its page (measured — `actionButton1` in 30 units of a blank project), so the predicate has to be a widget TYPE. Name it by the **MDL keyword**, resolved through the existing `pluggableKeywordIDs`, not by a `LIKE` over the stored id: `WidgetType LIKE '%datagrid%'` matches 20 widgets in 6 containers on a blank project because it sweeps in DatagridTextFilter/DateFilter/DropdownFilter, which do not carry the grid's design properties. **Reuse three things instead of growing a fourth of each** — `findMatchingWidgets` (the catalog query), the per-widget routing decision from the singular form, and `updateOutcome` from ako/mxcli#520 so a sweep that matches and writes nothing exits non-zero instead of claiming success. **Two ANTLR traps, both positional**: the rule has two `identifierOrKeyword` slots (optional module, WHERE value) returned as ONE list, so reading them positionally without checking `ctx.IN()` scopes a project-wide sweep to a module named after a widget type; and the sibling `ALTER PAGES … SET LAYOUT` shares the same prefix, so a test that the layout form still parses as itself is not optional. `ensureCatalog(ctx, true)` must be called before `findMatchingWidgets` or it nil-panics — a cold catalog otherwise reads as \"no such widgets\"","refs":["ako/mxcli#515","ako/mxcli#520"]} diff --git a/.claude/skills/fix-issue/findings/mdl-grammar.jsonl b/.claude/skills/fix-issue/findings/mdl-grammar.jsonl index 4c4b92ad02..459df95ad1 100644 --- a/.claude/skills/fix-issue/findings/mdl-grammar.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-grammar.jsonl @@ -57,4 +57,6 @@ {"area": "mdl/grammar", "date": "2026-09-08", "symptom": "Adding lexer tokens for a new feature broke an unrelated, previously-passing MDL example: `editable: never` on a list view stopped parsing after NEVER became a keyword", "cause": "A new lexer token steals every existing use of that word as an identifier or property value unless it is also added to the `keyword` rule in MDLSettings.g4. NEVER, ONLINE, SYNC and PRESERVE were added for offline sync; NEVER was already a real page property value", "file": "`mdl/grammar/domains/MDLSettings.g4` (keyword rule)", "insight": "Before adding a lexer token, grep the examples for that word as a value or name — the collision is with EXISTING scripts, so nothing in the new feature's own tests can find it. TestKeywordRuleCoverage catches the omission but only asserts the rule LISTS the token; add a test that the word still parses as an identifier, which is the property that actually matters. Here the two failures had one cause: the coverage test named the tokens and check-mdl named the victim file, and the file name (maint2-editable-never-create-page.mdl) said which word", "refs": ["PROPOSAL_offline_sync_configuration.md"]} {"area": "mdl/grammar", "date": "2026-09-15", "symptom": "Re-executing `describe workflow` output failed with `mismatched input 'boundary' expecting ';'` for any user task, call microflow or wait for notification that has two or more boundary events.", "cause": "formatBoundaryEvents emits `boundary event timer '…' { … }` per event (boundaryEventKeyword includes the prefix), and the syntax topic documents that per-clause form, but MDLWorkflow.g4 had `(BOUNDARY EVENT workflowBoundaryEventClause+)?` — one keyword, then clauses.", "fix": "All four sites accept `(BOUNDARY EVENT workflowBoundaryEventClause ((BOUNDARY EVENT)? workflowBoundaryEventClause)*)?`, so both the per-clause and the shared form parse; the visitor is unchanged.", "file": "mdl/grammar/domains/MDLWorkflow.g4", "insight": "A round trip that ends in `diff describe-1 describe-2` is vacuous when the exec in between fails: the second describe reads the unchanged document and matches. It reported IDENTICAL here while the exec had died on a parse error that a grep filter hid. Assert the exec itself — zero parse errors and a rewrite verb — before diffing. The integration round-trip tests had the same blind spot: they compare describe output but never feed it back to the parser, so a grammar/describer disagreement on a construct with more than one instance could not be seen. A test that re-parses describe output (TestWorkflowDescribe_TwoBoundaryEventsReparse) is the cheap guard."} {"area": "mdl/grammar", "date": "2026-09-18", "symptom": "Lint rule SEC005 reports \"strict mode is disabled\" and MDL has no statement that turns it on — the rule's own suggestion said \"not settable via MDL\". A lint rule with no remedy, recorded on the reporting project as the one finding left Open", "cause": "StrictMode was read everywhere and written nowhere: `security_read.go` reads it, `show security` prints it, the Starlark rule lints it, and `ProjectSecurity.SetStrictMode` existed in gen and was never called. `alterProjectSecurityStatement` had three variants (LEVEL, DEMO USERS, GUEST ACCESS) and no fourth", "file": "`mdl/grammar/MDLLexer.g4` + `domains/MDLSecurity.g4` + `domains/MDLSettings.g4` (keyword rule), `mdl/ast/ast_security.go`, `mdl/visitor/visitor_security.go`, `mdl/executor/cmd_security_write.go`, `mdl/backend/security.go`, `mdl/backend/modelsdk/security_write.go`, `mdl/backend/mock/mock_security.go`, `.claude/lint-rules/sec_strict_mode.star`", "insight": "**Writing a property gen merely offers is the trap; this is not one.** StrictMode is declared by BOTH generated sources and mxcli already reads it from real projects, which is the evidence that separates it from the Layout placeholder properties that make a document Studio Pro cannot open. **The AST field must be a POINTER** — a bare bool would disable strict mode on every DEMO USERS toggle, since \"said nothing\" and \"asked for off\" would be the same value (a test pins this). New tokens STRICT and MODE both go in the parser's `keyword` rule: `mode` is an entirely plausible attribute name and a keyword left out of that rule silently breaks every model already using the word (`TestKeywordRuleCoverage` catches it; a parse test pins it too). **Update the lint rule's suggestion in the same change** — a remedy that still says \"Studio Pro only\" leaves the finding exactly as unhelpful as before. No level-dependent refusal was added: the model stores StrictMode independently of SecurityLevel, and the rule already scopes its own advice to Production", "refs": ["#526"], "rules": ["SEC005"]} +{"area": "mdl/grammar", "date": "2026-09-20", "symptom": "`create snippet Test.SNIPPET_Label (params: { $Label: string }) { dynamictext dt (content: $Label) }` — the spelling `mxcli syntax snippet.create` printed in its own Syntax line — passed `mxcli check` and failed at exec with \"failed to build snippet: failed to resolve entity string: entity not found: string\", naming a type nobody spelled (mendixlabs/mxcli#1028).", "cause": "`snippetParameter`/`snippetParameterList` in MDLPage.g4 were a byte-identical duplicate of `pageParameter`/`pageParameterList` with their own visitor, buildSnippetParameterListAsPage, which never called buildDataType — so a primitive type never reached the AST and buildSnippetV3 (which had no primitive branch either) took the source text for an entity name. Collapsed: a snippet's Params clause IS pageParameterList, and buildPageParameters is the only conversion. The primitive is then REFUSED, not written: mxbuild rejects a primitive snippet parameter with CE0046, so writing one the way a page parameter writes one would have traded an unreadable exec error for a build failure.", "file": "`mdl/grammar/domains/MDLPage.g4` (duplicate rule deleted); `mdl/visitor/visitor_page_v3.go` (buildSnippetParameterListAsPage deleted); `mdl/types/snippet_parameter_types.go` (SnippetParameterTypeRule, the measurements); `mdl/executor/validate_snippet_parameters.go` (MDL087); `mdl/executor/cmd_pages_builder_v3.go` (buildSnippetV3 refusal, pageParamBSONType Long fix); `cmd/mxcli/syntax/features_page.go`; tests `mdl/executor/snippet_param_primitive_test.go`, `mdl/executor/validate_snippet_parameters_test.go`, `mdl-examples/bug-tests/1028-snippet-primitive-parameter{,.fail}.mdl`", "insight": "Two byte-identical grammar rules with two visitors is a bug generator, not a duplication smell: this clause produced TWO reported bugs from the same duplication in a fortnight (the quoted entity name, then this), and the first fix — patching the copy — left the second live and silent, turning a loud wrong error into a parameter with no type at all. When a fix is 'make X agree with Y' and X and Y are the same grammar, delete X. Second, and the reason step 6 of fix-issue is not optional: the obvious repair here (write the primitive the way a page parameter writes one) is supported by every source of truth in the repo — generated/metamodel declares Forms$SnippetParameter.ParameterType as the polymorphic DataTypes$DataType, exactly as Forms$PageParameter's, and the codec encodes it happily — and mxbuild rejects it with CE0046. A shape argument from the metamodel cannot see a validator rule. The control that made the rule crisp was putting the SAME six primitives on a PAGE in the SAME mxbuild run: six CE0046 on the snippet, 0 errors on the page, so the restriction is on snippet parameters and not on primitives, which is exactly what the error message now has to say. Third, a bug like this is a documentation bug as much as a code one — the reporter reached it by following `mxcli syntax snippet.create`, so a fix that leaves that line printing `$Label: String` re-creates the report. Aside found on the way: pageParamBSONType returned \"DataTypes$LongType\", a $Type that does not exist in gen OR generated/metamodel (constant_write.go had the note, 'storage has no LongType'), and pageParamTypeToGen's default arm quietly rescued it into a String — so a `Long` page parameter had been silently stored as String.", "ce": "CE0046", "rules": "MDL087", "refs": "mendixlabs/mxcli#1028; the sibling quoted-name fix in the same clause (mdl/visitor/snippet_param_quoted_entity_test.go); ADR-0005 guard-don't-drop"} {"area": "mdl/grammar", "date": "2026-09-21", "symptom": "A new settings option list keyed on `IDENTIFIER` makes the feature's ONLY option a parse error: `alter settings workflows add group 'Approvers' (Description: '\u2026')` \u2192 \"mismatched input 'Description' expecting IDENTIFIER\"", "cause": "`Description` is an MDL lexer keyword (DESCRIPTION, from the security statements), so it never matches IDENTIFIER. The rule was copied from `languageOption`, whose keys (CheckCompleteness, CustomDateFormat\u2026) all happen to be plain identifiers \u2014 so the pattern looked safe and was not", "file": "`mdl/grammar/domains/MDLSettings.g4` (`settingsItemOption`) + `mdl/visitor/visitor_settings.go` (`collectSettingsItemOptions`)", "insight": "Any `( key: value )` option list must key on `identifierOrKeyword`, not IDENTIFIER, and the visitor must read it with `unquoteIdentifier(ctx.IdentifierOrKeyword().GetText())`. Before writing one, grep MDLLexer.g4 for each key you intend to accept \u2014 the check costs seconds and the failure lands on the single statement the feature exists for. Copying an existing option rule proves nothing about your key set. Control: reverting the rule to IDENTIFIER fails TestAlterSettings_WorkflowGroup with exactly that message. mendixlabs/mxcli#272", "refs": ["mendixlabs/mxcli#272"]} +{"area": "mdl/grammar", "date": "2026-09-22", "symptom": "A `create workflow` clause written in the \"wrong\" position is a parse error — `on created microflow` anywhere but between the targeting clauses and `entity` gives `line 6:4 mismatched input 'ON' expecting ';'`, and a header clause out of place gives `mismatched input 'DISPLAY' expecting {ON, BEGIN, EXPORT, DUE, OVERVIEW}`. Neither names the clause or the rule, and one misplaced clause cascades into 3–7 more errors including a bogus `extraneous input 'END'`. The reporter reverse-engineered the order empirically and wrote it into their notes", "cause": "`createWorkflowStatement` and `workflowUserTaskStmt` were a fixed SEQUENCE of optional groups — each clause optional, its POSITION not — and the VISITOR depended on that: it read qualified names by COUNTING (`names[1]` or `names[2]` for the overview page depending on whether PARAMETER was present; `nameIdx` walked page → targeting → on-created → entity) and strings by index off `AllSTRING_LITERAL()`. So the grammar could not simply be relaxed", "file": "`mdl/grammar/domains/MDLWorkflow.g4` (new `workflowHeaderClause`, `workflowUserTaskClause`, `workflowMultiUserTaskClause`), `mdl/visitor/visitor_workflow.go` (`applyWorkflowUserTaskClause`), `mdl/visitor/visitor_workflow_clauses.go` (`checkWorkflowClausesAtMostOnce`)", "insight": "**Positional reading is what makes a clause order load-bearing, so the grammar fix is a visitor fix.** The tell is `names[idx++]` in an exit-listener: the rule already carried a comment warning that reading strings by position had nearly mis-assigned FOLDER, and the same hazard had simply been left standing for qualified names. **A clause set must re-add the at-most-once rule the sequence gave for free**, or `page M.A page M.B` starts parsing with the second silently winning — a worse failure than the parse error it replaces. Enforce it in the visitor, not the grammar: only there can the message say `duplicate PAGE clause on user task Review (already given on line 12)`. **Two spellings that fill one model slot are ONE clause**: `targeting microflow` + `targeting xpath` were both accepted and the LAST one won, though a user task stores one UserSource — order-dependence in its most damaging form, and now a duplicate. **Keep the MULTI alternative's own clause rule rather than collapsing to `MULTI?`** — relaxing the order must not relax the vocabulary, or a single user task starts accepting `decide by`. **Control that settles it**: build a `bin/mxcli` from HEAD in a `git worktree`, exec the canonical-order script with it, and compare the written `.mxunit` against the fixed binary's output for BOTH orders — 6,038 bytes each, identical in every string ≥8 chars, differing only in the randomly minted element `$ID`s. AST `reflect.DeepEqual` between the two orders is the unit-level version of the same claim; both-parse is not enough, since a relaxed grammar over a positional visitor parses and mis-assigns. Found in passing and NOT fixed here: `create workflow … overview page X` writes nothing (`mdl/backend/modelsdk/workflow_write.go` has no `OverviewPage`), while `alter workflow … set overview page` does. Tests `mdl/visitor/visitor_workflow_clause_order_test.go`; repro `mdl-examples/bug-tests/workflow-586-clause-order.mdl` with its `-canonical.mdl` control and `-duplicate-clause.fail.mdl` sibling", "refs": ["ako/mxcli#586"]} diff --git a/.claude/skills/fix-issue/findings/modelsdk.jsonl b/.claude/skills/fix-issue/findings/modelsdk.jsonl index 59c630c7bb..61eef19a39 100644 --- a/.claude/skills/fix-issue/findings/modelsdk.jsonl +++ b/.claude/skills/fix-issue/findings/modelsdk.jsonl @@ -17,3 +17,5 @@ {"area": "modelsdk", "date": "2026-09-02", "symptom": "Every in-place edit of a page is refused with `refusing to write unit \u2026: 1 element id(s) are used more than once \u2026 held by [Texts$Translation \u00d78]` \u2014 `GRANT VIEW ON PAGE`, `ALTER PAGE \u2026 INSERT` \u2014 while a full `CREATE OR REPLACE PAGE` still works, so the page looks correct and only UPDATES are blocked. Surfaces after a second language is enabled.", "cause": "`canon.CarryTranslations` pairs a rebuilt text to its stored translations BY SOURCE STRING when the two documents' text paths differ, and `mergeText` appended the stored `Texts$Translation` element **verbatim** \u2014 deliberately, because keeping the stored `$ID` is what lets no-op elision fire. When several rebuilt texts share one source string (eight copies of the literal `'{1}'` on a page is ordinary), all of them resolve to the SAME stored set and every one got the same element, id included. `reuseSafeID` now gives the first use the stored id and derives a fresh deterministic one (SHA-256 of stored id + containment path + language) for each further copy; the visit order is sorted rather than map order, or which text keeps the stored id would vary per run and the document would churn.", "file": "`modelsdk/canon/translations.go` (`reuseSafeID`, `derivedID`, `elementIDs`, `sortedPaths`, `mergeText`), `modelsdk/canon/duplicates.go` (comment corrected \u2014 it recorded the cause as unestablished)", "insight": "**Re-identifying a copy is safe here in a way that deduplicating ids in general is not, and that distinction is the whole argument.** An `$ID` is a pointer target and rewriting one means finding every reference (ADR-0008) \u2014 which is exactly why `duplicates.go` refuses rather than repairs. Nothing references a `Texts$Translation`: it is a leaf child of a `Texts$Text` with four keys and no identity anything resolves by, so there are no references to miss. Only the COPIES are re-identified; the first use keeps the stored id, so an unchanged document still compares equal. **Verify elision explicitly after touching this** \u2014 the fix trades against the exact property the verbatim append existed for: measured, a second identical run still reports `Unchanged page` with the same sha and mtime. Controls, end-to-end on a real 11.13 project with de_DE enabled and three widgets sharing a caption: the pre-fix binary writes one id used 3\u00d7 and the next `ALTER PAGE` is refused with the reporter's message verbatim; the fixed binary writes 27 distinct ids for 27 elements, the `ALTER PAGE` succeeds, the German translation survives (the control against a 'fix' that just stops carrying), and `mx check` is 0 errors. Reported as CapTrackV2 FINDINGS \u00a730/\u00a717."} {"area": "mdl/backend/modelsdk", "date": "2026-09-07", "symptom": "`mxcli lint` QUAL002 reported \"Page 'X' has no documentation\" against a page carrying a javadoc comment; the catalog's Description column was blank for every page and snippet; `describe page` emitted no documentation. The comment looked, from every angle, like it had been dropped (ako/CapTrackV4 R12).", "cause": "Nothing was dropped: the AST, executor and writer all carry it, and `mxcli bson dump --type page` shows Documentation with the right value. pageFromGen and the ListSnippets constructor in mdl/backend/modelsdk/page.go simply did not read it back, so on the DEFAULT engine every symptom downstream of the read was wrong at once. Fixed by carrying Documentation in both. Separately, QUAL002 stopped sweeping modules: a Mendix module HAS no documentation property (generated/metamodel's ProjectsModule declares none, modelsdk/gen's Module has no accessor, no stored Projects$ModuleImpl carries the key).", "file": "`mdl/backend/modelsdk/page.go` (pageFromGen, ListSnippets); `mdl/linter/context.go` (documentableSources); `.claude/lint-rules/missing_documentation.star`; tests `mdl/backend/modelsdk/page_documentation_test.go`", "insight": "When a value looks absent everywhere, check the WRITE first: `bson dump` showed it stored correctly and localised the bug to the read in one step, where chasing the reported symptom would have started at the visitor. The engine split is the second cheap discriminator — the legacy reader parsed it fine, so the defect was in the default engine alone. A stale catalog nearly hid that: an earlier per-engine comparison reused a cached catalog.db and showed both engines empty, so DELETE the catalog between engine comparisons rather than trusting `refresh catalog full`. Finally, page and snippet were 2 of 5 sibling readers in one file — layout, building block and page template all carried Documentation — which is the shape to look for when one document type behaves differently from its neighbours. And a rule asking for a property the platform does not have is not a gap in the language: three sources agreed before that row was removed."} {"area": "modelsdk/meta", "date": "2026-09-22", "symptom": "A view entity selecting `u.Name` from System.User could not be declared in any way that both passed `mxcli check` and built: `String(100)` (the correct length) was refused, `String` (unlimited) passed check and then failed mxbuild with CE6770 \"View Entity is out of sync with the OQL Query\". `describe entity System.User` reported `Name: String(unlimited)`. Reported in ako/ChipCoV4 FINDINGS.md against Mendix 11.14.0 (ako/mxcli#584, with #585 the other half).", "cause": "meta.SystemAttrDef declared a Length field and NOT ONE of the 115 String attributes in modelsdk/meta/system_module.go populated it, so systemAttrType built every System string as StringAttributeType{Length: 0} — which mxcli reads as unlimited. Every length comparison against a System attribute was therefore made against 0. Fixed by measuring all 115 and populating them, with a golden table (modelsdk/meta/testdata/system_string_lengths.txt) and TestSystemStringLengths holding the two in step.", "file": "`modelsdk/meta/system_module.go` (SystemEntities, SystemAttrDef.Length); `modelsdk/meta/testdata/system_string_lengths.txt`; tests `modelsdk/meta/system_string_lengths_test.go`, `modelsdk/meta/system_string_lengths_measure_test.go`, `mdl/backend/modelsdk/system_module_read_test.go`", "insight": "The System module's attribute lengths are IN THE BUILD OUTPUT: `deployment/model/model.mdp` is a stream of BSON documents (each with its own 4-byte length prefix — unmarshalling the file whole fails with \"invalid document length\"), the System module arrives as a Projects$ModuleImpl carrying only a Name with its DomainModels$DomainModel immediately after, and every entity's attributes are there with their StringAttributeType.Length. That is ONE `mxbuild --target=deploy` for all 115, and it is the model the runtime builds the tables from, so it is the same number CE6770 is decided by. Two searches not worth repeating, both spent on this issue: the Mendix Model SDK does not carry them (its gen/ describes metamodel TYPES, so System.User.Name is not in it) and the modeler's own copy is inside Mendix.Modeler.Core.dll, i.e. a decompiler. The one-view-entity-per-attribute mxbuild probe works but is ~40s each. The version question answers itself the same way: building 10.24.4.77222 as well showed all 216 shared attributes identical in type AND length to 11.14.0, so one table serves every supported version instead of a per-version registry — measure the second version rather than reasoning about it, it is one more build. Finally, 0 is Mendix's own encoding of \"unlimited\" (46 of the 115), so populating the table does not make 0 safe to read as a length — what makes it safe is that the golden enumerates every String attribute, so 'unmeasured' cannot exist without failing a test."} +{"area": "docs / modelsdk/mpr", "date": "2026-09-22", "symptom": "Four reference pages described an MPR v1 `UnitContents` table holding the BSON blobs, and a v1/v2 detection recipe that probes for it. No .mpr has ever had that table: a v1 file has exactly `Unit` and `_MetaData`, and contents are the `Unit.Contents` blob. `grep -rn UnitContents --include=*.go` is 0 hits. Reported by an outside reader building an independent format reader (mendixlabs/mxcli#1072).", "cause": "Never-measured prose. The pages also invented `UnitType` and `Name` columns on `Unit` (there are seven columns and neither is among them — type and name come out of the BSON `$Type`/`Name`), and drew `mprcontents/` flat when it is sharded `//.mxunit`. Fixed by rewriting the four pages from the SQLite catalogs of two real fixtures, and adding modelsdk/mpr/docs_schema_test.go to hold them there.", "file": "`docs-site/src/internals/mpr-format.md`, `docs-site/src/internals/mpr-v1-v2.md`, `docs-site/src/appendixes/version-compatibility.md`, `docs/05-mdl-specification/10-bson-mapping.md`; test `modelsdk/mpr/docs_schema_test.go`", "insight": "Prose cannot be type-checked but the IDENTIFIERS in it can, and the rule that makes it zero-maintenance is a prefix rule, not an allowlist: check only names BEGINNING with a real table name (`Unit`, `_MetaData`, `_Transaction`) against the union of the fixtures' tables and columns. `UnitContents` and `UnitType` are caught; the catalog tables these same pages mention (`REFS` and friends) never start with a real .mpr table name, so they need no exemption and no one has to maintain a list. The page set is discovered by content (any .md under docs-site/src or docs/05-mdl-specification mentioning `.mpr`/`mprcontents`), so a page added later is covered without anyone remembering. One consequence worth stating in the docs themselves: a page that wants to say a column does NOT exist must say it in PROSE — the first fix wrote \"there is no `UnitType` column\" and the test flagged its own remedy, which is correct, because the old pages' \"no `UnitContents`\" at mpr-v1-v2.md:35 read as a v1/v2 difference rather than as a fiction and an exemption for denials would have masked it. The control is cheap and exact here: `git stash` the doc edits with the test file kept, and the failures reproduce the reporter's line list verbatim (version-compatibility.md:31, mpr-format.md:21,23, mpr-v1-v2.md:12,35,69,73,74,84,94, 10-bson-mapping.md:30) plus the two they had not found. No Mendix tool runs in this fix's argument — the claims are about SQLite schema and are read straight off `sqlite_master`/`PRAGMA table_info`, which is the primary source, so the usual 'build two apps' rule does not apply.", "refs": ["mendixlabs/mxcli#1072"]} +{"area": "docs / modelsdk/mpr", "date": "2026-09-22", "symptom": "The MPR reference pages' \"Unit Types\" tables mapped BSON `$Type` to document kinds, and 15 of the rows named a spelling no unit carries: `Pages$Page`/`Pages$Layout`/`Pages$Snippet`/`Pages$BuildingBlock` (real units say `Forms$*`), and docs/05-mdl-specification/10-bson-mapping.md lowercased eleven more (`microflows$microflow`, `pages$page`, `security$ProjectSecurity`…). It also listed `CustomWidgets$customwidget` as a document type. Found while fixing mendixlabs/mxcli#1072, filed and fixed separately.", "cause": "The tables were written from the TypeScript SDK's QUALIFIED names rather than the storage names Mendix writes — the same split CLAUDE.md documents for `ShowPageAction`/`ShowFormAction`, never applied here. `CustomWidgets$CustomWidget` is a widget element inside a page's tree (mdl/catalog/builder_widget_refs.go), never a unit, so that row was removed rather than corrected.", "file": "`docs-site/src/internals/mpr-format.md`, `docs/05-mdl-specification/10-bson-mapping.md`; test `modelsdk/mpr/docs_schema_test.go` (TestDocumentedUnitTypesUseStorageNames)", "insight": "Measuring the real set is one command and settles the whole table at once: decode every `mprcontents/*/*/*.mxunit` (and every v1 `Unit.Contents` blob) and count `$Type` — 28 distinct values across a blank 11.6.6 app and a 9.24.30 one. Do NOT try to verify rows one at a time against gen, which carries BOTH spellings: `model/types.go` defines `DocumentTypePage = \"Pages$Page\"` and mdl/catalog/builder_xpath.go defensively matches `Forms$Page` AND `Pages$Page`, so grepping the codebase 'confirms' the wrong name. The fixture is the arbiter; the codebase is not. The test rule that makes this checkable without a maintenance burden keys on the LOCAL name after the `$`, case-insensitively: a fixture cannot prove a type ABSENT (a blank project has no business-event service), so demanding every documented type be present would fail correct rows — but when the fixture has a type with the same local name, the documented row must equal it exactly. That catches all four `Pages$` rows and all eleven lowercase ones with zero false positives. Its stated limit is real and cost a manual fix: a row whose local name appears nowhere in the fixtures is not checked at all, which is how `CustomWidgets$customwidget` slipped past and had to be removed by hand. One editing trap, not a Mendix one: anchoring a section replacement on `'---'` matches a markdown TABLE SEPARATOR (`|---|---|`) long before the horizontal rule you meant — the edit silently no-ops on the table you were replacing. Anchor on `'\\n---\\n'`.", "refs": ["mendixlabs/mxcli#1072"]} diff --git a/.claude/skills/fix-issue/findings/other.jsonl b/.claude/skills/fix-issue/findings/other.jsonl index 51edfa9882..314922aa01 100644 --- a/.claude/skills/fix-issue/findings/other.jsonl +++ b/.claude/skills/fix-issue/findings/other.jsonl @@ -16,3 +16,6 @@ {"area": "web/dist", "date": "2026-08-30", "raw": "| After `mxcli test … --local`, an app another `mxcli run --local` is serving goes blank while still answering HTTP 200 (~1.7 KB, the Mendix SPA shell); the runtime log shows `Connector: 404 - file not found for file: dist%2Findex.js` and `deployment/web/dist` is gone | `cmd/mxcli/testrunner/localapp_options.go`, `cmd/mxcli/testrunner/runner_local.go` (`localTestDeployDir`), `cmd/mxcli/testrunner/runner.go` (`checkScratchDeploymentExists`) | A local test run already used its own ports and its own `_test` database — the code comment says why, verbatim — but shared the **deployment directory**, which is the one the *browser* reads. A headless test boot does not bundle the web client, so its build left the running app serving the shell over a 404: tests pass, run keeps running, app is blank, nothing reported at either end. **Detection was not the fix**: the two processes use different ports by design, so no port check can see it, and a lock file would only turn a silent blanking into a refusal. The test boot now builds into `/.mxcli/deployment-test/` — gitignored, already where the test runtime log lives — which makes the collision impossible. Note booting a runtime against the shared directory damages it even **without** a rebuild (the packaging step removes the bundle — FINDINGS §35, `ReportLostWebClientBundle`), so \"reuse the dev loop's tree read-only\" is not an alternative. Consequence to wire: `--skip-build` used to mean \"reuse deployment/\" and now has nothing until tests have run once, so it is refused with the reason rather than failing inside the runtime boot against a path the user never chose. Reported as mxcli-formula1 FINDINGS §62 |"} {"area": ".claude/skills/mendix/record-narrated-demo", "date": "2026-09-13", "symptom": "In a narrated demo recorded with CSS `zoom` (take.js's fix for a fixed-width Mendix page), narrate.js's `point()` highlight ring is drawn around the wrong control or off the edge of the frame, and the caption plate is the wrong height and sits outside the film's caption band. Nothing in the take, the beat assertions or the contact sheet reports anything.", "cause": "Under `html{zoom:z}` Chromium reports `getBoundingClientRect()` in ZOOM-ADJUSTED (video) pixels but `getComputedStyle()` and `style.*` in CSS pixels. `point()` read a rect and assigned it straight to `style.left/top/width/height`, so the ring landed at position x z (measured at z=1.6842: a target at (168,202) ringed at (274,330)). The plate had the mirror-image problem: its geometry was declared in CSS pixels, so a 96px bar reached the file as 96 x z = 162 video px against a 184px caption band.", "file": ".claude/skills/mendix/record-narrated-demo/narrate.js (`point`, `css`, `checkOverlay`)", "insight": "The overlay lives in the page's coordinate space and the film is specified in the frame's, and `zoom` is the only conversion between them - so every overlay number is now stated in VIDEO pixels and divided by a zoom passed to `configure()`. The trap is that the conversion runs in opposite directions depending on which API you read it back with, which is why the fix came with `checkOverlay()`: it measures the installed plate against the band and refuses the take, with the control being one line (build the overlay without telling it the zoom -> 'caption plate is 310 video px tall, the band is 184'). A design rule that can be measured in the page should be a check that throws at record time, not a note in a skill - the same argument PRODUCTION.md sec 12 makes for compositions.", "refs": ["ako/mxcli-intro-video video-system/DESIGN-LANGUAGE.md"]} {"area":".claude/skills/packs","date":"2026-09-21","symptom":"A URL-fed Vega-Lite chart in the mendix-vega-charts pack drew its axes and a FULL legend with zero data points, no console error and no Vega warning. The skill stated that \"same-origin requests carry the session cookie, so an endpoint authenticated by session is reachable ... without any token handling\".","cause":"Mendix refuses a session-authenticated request without the session's CSRF token on READS too, not just writes and not just /xas/. The cookie is sent; it is not sufficient. Vega's loader read the 401 body as an empty dataset, so the failure surfaced as a plausible-looking empty chart rather than as an error.","file":".claude/skills/packs/mendix-vega-charts/widget/src/csrf.ts, .../SKILL.md, cmd/mxcli/skillpacks_test.go","insight":"THE LEGEND IS THE TELL: it is built from the spec's scales, not from rows, so a chart with a complete legend and no marks has had its DATA refused, while a chart with a broken legend has a spec problem. That one distinction separates the two hypotheses before any measurement. Two things then send the diagnosis the wrong way and cost the time: document.cookie shows only originURI=/login.html (XASSESSIONID and xasid are httpOnly, so the browser IS sending them and JavaScript cannot see them) and basic auth on the same URL returns the data, which reads as proof the endpoint is fine and the chart is broken. Isolate on the HEADER, not the URL: two requests, one added header, everything else equal -- 401 vs 200, measured on a fresh 11.14.0 app. Skip the plausible wrong turn of adding the header unconditionally: the issue's own suggested loader tests the URI with /^[a-z][a-z0-9+.-]*:\\/\\//i, which passes //elsewhere.example/rows.json (no scheme, another host) and hands that host a working session credential. Resolve with new URL(uri, base) and compare origins instead -- it also gets the converse right, an absolute URL naming the app's own origin IS the app. End-to-end control through vega's real loader against the running app: 0 marks / 3 axes / no error without the token, 1 mark with it, which reproduces the reported symptom exactly.","refs":["ako/mxcli#574"],"ce":[],"mendix":"11.14.0"} +{"area": "skills", "date": "2026-09-22", "symptom": "A workflow inbox over System.WorkflowUserTask, re-sourced from a MICROFLOW to get past the System-module ceiling, drew the right number of cards and every card was COMPLETELY BLANK — no CE code, no console warning, and mxcli check, lint, report and docker check all at 0 errors. The manage-security and system-module skills had offered exactly that microflow data source as the way past the ceiling (ako/ChipCoV4, Mendix 11.14.0; ako/mxcli#587).", "cause": "Not an mxcli defect — a Mendix rule both skills stated as a workaround without having measured it. A microflow does not apply entity access, so its retrieve returns every row, but the runtime RE-APPLIES entity access when it serializes those objects to the client, XPath constraint included: a row the role may not read arrives with every member empty. Both skills now state the rule (\"a microflow data source moves the ROWS, not the MEMBERS\") with the measurement, and point at reading the member inside the microflow and returning a module-owned object.", "file": "`.claude/skills/mendix/manage-security/SKILL.md` (The System-module ceiling); `.claude/skills/mendix/system-module/SKILL.md`; measurement `mdl-examples/bug-tests/security-587-system-member-access.mdl`", "insight": "The probe that settles this in one page: TWO microflow-sourced lists over the SAME retrieve — one over the System objects, one over a module-owned copy whose attribute was read inside the microflow — opened by an Administrator and by a plain User. The row COUNTS are the discriminator and they are equal in all four cells (2 and 2), which is what proves the microflow moved the rows and isolates the loss to serialization; only the System list loses values, and only for the non-admin. Use System.User rather than System.WorkflowUserTask for the probe: it needs no workflow and it shows the mechanism MORE sharply, because System.User's own rule reads [id = '[%CurrentUser%]'] so the non-admin sees exactly one populated row and one blank — per-OBJECT blanking, not per-attribute. That also explains why a user picker 'lists the current user only' and a workflow inbox is entirely blank: same rule, different constraint. The control here is the ROLE, not a before/after build — same binary, same model, two logins — so no A/B rebuild is needed. Two traps in the probe itself: dynamictext content is a static template and renders '[%Name%]' literally, so bind the attribute with a TEXTBOX; and `grant on System.User` PASSES `mxcli check --references` and is refused only by `exec`, so a script carrying one checks clean and then stops part-way."} +{"area": "ci", "date": "2026-09-22", "symptom": "A failing CI job's log held nothing but `##[error]Process completed with exit code 1` — no test name, no failure message. On `windows-process-regression` that made a red check impossible to diagnose from the log alone: the only evidence of WHICH test had failed was the runner's own `Terminate orphan process: pid (2760) (PING)` cleanup line, absent from the green run on main.", "cause": "The step captured the command into a variable — `out=$(go test -v -run '…' ./cmd/mxcli/docker/)` — and echoed it on the NEXT line. The runner's shell is `bash --noprofile --norc -e -o pipefail`, so a non-zero `go test` aborts the step AT THE ASSIGNMENT and the `echo \"$out\"` never runs. The capture existed only to count `--- PASS:` lines (a `-run` filter passes vacuously if the tests are renamed away). Replaced with `go test … 2>&1 | tee go-test-output.txt`, `status=${PIPESTATUS[0]}`, then grep the file — output streams as it is produced, the vacuous-run guard still counts, and a real failure is reported with its own exit status. Same pattern was in `tunnel-seam-cross-platform`; both fixed.", "file": "`.github/workflows/push-test.yml` (tunnel-seam-cross-platform, windows-process-regression)", "insight": "**A CI step that captures output to echo it later loses exactly the runs you need it for** — it prints on success, where nobody reads it, and prints nothing on failure. `set -e` is what makes it silent, so it looks fine in local testing without `-e`. Grep workflows for `=$(` around a build/test command before trusting a bare exit code. The measurement that settles it costs a minute and needs no CI: extract the step's `run:` block straight out of the YAML (`yaml.safe_load`), put a stub `go` on PATH that exits 1 with realistic output, and run the block under `bash --noprofile --norc -e -o pipefail` — the old body prints zero lines. Exercise the vacuous-`-run` case too, or the fix quietly disables the guard the capture was there for.", "refs": ["ako/mxcli#594"]} +{"area": "examples/doctype-tests", "date": "2026-09-22", "symptom": "The nightly fails on Mendix 10.24 only, in `TestMxCheck_DoctypeScripts/14-project-settings-examples.mdl`, with `Execution error: alter settings workflows add group '' requires Mendix 11.2.0+ (project is 10.24.24.119349)`. Push CI (single, newer version) and `TestDoctypeScriptsParseAfterVersionFiltering` both pass", "cause": "The workflow-groups feature (832e9c80) added Examples 4.3-4.7 to the doctype script ungated, although its executor correctly refuses the statement below 11.2 (`Settings$WorkflowGroup` is 11.2 metamodel). Third time this class has landed (Atlas building block in 15c, `DecimalScale`, now workflow groups)", "file": "`mdl-examples/doctype-tests/14-project-settings-examples.mdl`", "insight": "Wrap the examples in `-- @version: 11.2+` ... `-- @version: any`, with the directive ABOVE the first `/** */` comment. The parse-only guard cannot catch this: it proves the filtered script parses, not that the executor accepts every statement on that version, so a version-refused statement only surfaces in the nightly's 10.24 job. When a feature adds a version check to the executor, gate its doctype example in the same change. Reproduce locally in about 30s: `mxcli setup mxbuild --version 10.24.24.119349`, then `go test -tags integration ./mdl/executor/ -run 'TestMxCheck_DoctypeScripts/