Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/lint-rules/data_change_microflows.star
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def check():
# Check each persistent entity
for entity in entities():
# Skip non-persistent and view entities (they don't need data-change microflows)
if entity.entity_type != "PERSISTENT":
if entity.entity_type != "Persistent":
continue

# Get all references to this entity
Expand Down
2 changes: 1 addition & 1 deletion .claude/lint-rules/entity_business_key.star
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def check():

for entity in entities():
# Skip non-persistent entities (they don't need business keys)
if entity.entity_type != "PERSISTENT":
if entity.entity_type != "Persistent":
continue

# Skip system/administration entities that typically use internal IDs
Expand Down
1 change: 1 addition & 0 deletions .claude/skills/fix-issue/findings/cmd-mxcli.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
{"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/<tag>/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", "date": "2026-09-22", "symptom": "ARCH002 and ARCH003 never report anything. `mxcli lint` runs them, prints no error and lists them under --list-rules, yet a project whose entities have no business key and no data-change microflow comes back clean.", "cause": "Both rules guard with `if entity.entity_type != \"PERSISTENT\": continue`. The catalog column stores PERSISTENT, but LintContext.Entities normalizes it (CASE WHEN 'PERSISTENT' THEN 'Persistent') before Starlark sees it, so the guard is true for every entity and the body never runs. The write-lint-rules skill documented a third spelling again, \"persistent\", in both its field table and its worked example.", "file": ".claude/lint-rules/entity_business_key.star, .claude/lint-rules/data_change_microflows.star, mdl/linter/context.go", "insight": "A Starlark rule that compares a field to a value that never occurs fails open: no error, no output, and --list-rules still shows it, so it looks like a project with nothing to report. The literals a rule may compare against are worth asserting in a test that loads the shipped rule file itself; measured on a generated app, fixing one spelling took ARCH003 from 0 findings to 5.", "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"]}
4 changes: 2 additions & 2 deletions .claude/skills/mendix/write-lint-rules/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def check():
| `qualified_name` | string | `"Sales.Customer"` |
| `module_name` | string | `"Sales"` |
| `folder` | string | `"DomainModel"` — folder path within module |
| `entity_type` | string | `"persistent"`, `"NonPersistent"`, `"view"` |
| `entity_type` | string | exactly `"Persistent"`, `"NonPersistent"` or `"View"` — any other spelling matches nothing and the rule silently reports nothing |
| `description` | string | Documentation text |
| `generalization` | string | Parent entity qualified name |
| `attribute_count` | int | Number of attributes |
Expand Down Expand Up @@ -475,7 +475,7 @@ SEVERITY = "warning"
def check():
violations = []
for e in entities():
if e.entity_type == "persistent" and not e.is_external and e.access_rule_count == 0:
if e.entity_type == "Persistent" and not e.is_external and e.access_rule_count == 0:
violations.append(violation(
message="persistent entity '{}' has no access rules".format(e.qualified_name),
location=location(module=e.module_name, document_type="entity", document_name=e.name),
Expand Down
Loading
Loading