Sync ako/mxcli: check-time fixes for #1100–#1104, and describe → exec fidelity

.claude/skills/fix-issue/findings/cmd-mxcli.jsonl

@@ -109,3 +109,5 @@
 {"area": "cmd/mxcli", "date": "2026-09-13", "symptom": "`build-and-test` fails in CI on `TestSettleSourceReturnsPromptlyForOneChange` \u2014 \"a quiet source took 196.975373ms to settle, want under 100ms\" \u2014 while the SAME tree passes in another run of the same workflow minutes earlier", "cause": "The test bounded elapsed wall-clock time as a multiple of the poll interval (`poll * (sourceSettleWindow + 3)`, 100ms against a nominal 40ms). settleSource waits on `time.After(poll)`, which guarantees AT LEAST the duration and nothing about the upper bound, so a loaded runner blows the budget with no defect present.", "file": "cmd/mxcli/docker/runlocal.go (settleSourceWith, the injected tick), cmd/mxcli/docker/runlocal_settle_test.go", "insight": "The property being guarded was a POLL COUNT, not a duration \u2014 'a quiet source costs one extra poll' \u2014 so the fix is to make polls countable (inject the timer) rather than to widen the budget, which only moves the flake threshold. Diagnosis shortcut worth reusing: the same workflow ran twice on the same tree, once from the push event and once from the pull_request merge commit, and disagreed \u2014 two runs of one tree is direct evidence of nondeterminism and cheaper than reading the test. Two things the controls settled that reasoning did not: (1) the assertions are written in terms of `sourceSettleWindow`, so WIDENING that constant leaves both tests green \u2014 they assert the loop honours whatever window is declared, never the number itself, and the real control is a loop that costs one poll MORE than it declares (both fail). (2) Each tick call must return a freshly-armed channel; returning one shared channel makes the multi-file test HANG rather than miscount, so the re-arm is load-bearing and not a style choice. The seam also made a previously untestable guarantee expressible: the window must be sourceSettleWindow CONSECUTIVE quiet polls, and dropping `quiet = 0` from the change branch was green against every pre-existing test in the file.", "refs": ["ako/mxcli#449"]}
 {"area":"cmd/mxcli","date":"2026-09-15","symptom":"Porting cmd/mxcli/docker off sdk/mpr moved two WRITE paths (ensureDemoUsers, applyHarvest) onto the codec backend. A baseline diff of `docker check` showed the project byte-identical across 421 files — which proved nothing, because the run had not written anything.","cause":"docker check's widget-update harvest is a no-op on an already-clean fixture, so an output+filetree diff against a pre-port binary exercises only the READ paths. Coverage then showed ensureDemoUsers at 0.0% — a write path the port touched that no test in the package ran.","file":"cmd/mxcli/docker/build.go","fix":"Added TestEnsureDemoUsers_CreatesAdminWhenNoneExist and _SkipsWhenUsersExist, plus a clearDemoUsers helper that establishes the precondition. Coverage 0.0% -> 76.5%. The read paths keep the baseline-diff evidence; applyHarvest was already at 76.9% via TestRunUpdateWidgets_RestoresV2AfterConversion.","insight":"A byte-identical baseline diff is strong evidence for a READ port and near-worthless for a WRITE port, because the natural control (nothing changed) is also what a no-op produces. The two need different instruments, and the cheap way to tell which you have is `go test -coverprofile` + `go tool cover -func` grepped for the functions you touched: it answers 'did my port's code even run' in one command, where a passing suite does not. Here it separated applyHarvest (76.9%, genuinely exercised including its UpdateRawUnit) from ensureDemoUsers (0.0%) inside the same package, so the gap was specific rather than a general absence of tests. Second trap, hit while fixing it: the shared v2 fixture ALREADY HAS two demo users, so the create-path test skipped and the idempotence test asserted the wrong count. Skipping on an unmet precondition is the #808 shape — set the precondition up instead (RemoveDemoUser in a helper, then assert the helper actually emptied it before proceeding). Third: read back through a FRESH connection, since asserting on the value the writer still holds passes against a write that never reached disk."}
 {"area":"cmd/mxcli","date":"2026-09-15","symptom":"Porting the last cmd/mxcli readers off sdk/mpr, cmd_extract_templates.go compiled with a type error (RawType/RawObject are bson.D on sdk/mpr, any on types.RawCustomWidgetType). Casting past it would have compiled — and broken the command at runtime, because FindCustomWidgetType is UNIMPLEMENTED on the codec backend.","cause":"mdl/backend/modelsdk/unimplemented_gen.go carries FindCustomWidgetType; measured at runtime it returns 'FindCustomWidgetType is not implemented on the model engine. This should be unreachable'. cmd_extract_templates.go was calling it through a concrete *mpr.Reader, so it was reachable only by NOT going through the backend.","file":"cmd/mxcli/cmd_extract_templates.go","fix":"Left this one file on sdk/mpr with a comment saying why and what would fix it (implement FindCustomWidgetType on the codec backend), and ported the other five. cmd/mxcli is otherwise clean; importers 13 -> 8.","insight":"The type error was the lucky part. A compile error is the ONLY reason this did not ship as a runtime failure — the cast that silences it is one line, and nothing else would have objected. When a port hits a type mismatch at a backend boundary, check whether the backend method is implemented at all before reconciling the types: `grep -n '<Method>' mdl/backend/modelsdk/unimplemented_gen.go` answers it in one command, and a runtime probe (connect read-only, call it, log the error) confirms it in under a minute. Note the direction of the trap: the unimplemented method's own error says 'This should be unreachable', and porting a caller to the backend is precisely what MAKES it reachable — so the #477 census blind spot (callers holding a concrete reader are invisible) cuts both ways. Second, smaller measurement trap in the same slice: a baseline diff of `check --post-migration` showed 50 lines vanishing, which looked like a regression and was not — the FIRST run built and cached a catalog inside the project, so the second run reused it. Two binaries must each get their own fresh copy of the fixture, exactly as for a write port; a command that caches into the project directory makes consecutive runs non-independent even when nothing is being written on purpose."}
+{"area": "cmd/mxcli", "date": "2026-09-16", "symptom": "mendixlabs/mxcli#1103: a RETRIEVE with LIMIT inside a .test.mdl block was reported as `mismatched input 'LIMIT' expecting {GROUP_BY, SELECT, HAVING}` — the OQL follow set — on the statement `mxcli syntax microflow.retrieve` prints as its own example. The reporter concluded the test-block path routes microflow statements into the OQL parser.", "cause": "It does not. The generated MxTest.Test_* microflow parses fine (measured end-to-end against a real 11.6.6 project: the flow was created with the LIMIT intact). The message came from `mxcli check`/the LSP being pointed at the .test.mdl file itself, which they parsed as top-level MDL. A test block is a MICROFLOW BODY: DECLARE is not a top-level statement, the parser resyncs, RETRIEVE is a NON-RESERVED keyword so it is swallowed as an identifier, and the leftover `FROM …` starts oqlQueryTerm's FROM-first alternative (mdl/grammar/domains/MDLCatalog.g4), whose follow set is exactly {GROUP_BY, SELECT, HAVING}.", "file": "cmd/mxcli/testrunner/check_source.go", "fix": "testrunner.CheckSource renders each block as the microflow it becomes, padded so every body keeps its SOURCE line numbers (wrapper fragments go on the lines the doc comment and the '/' separator occupied). cmd_check.go and lsp_diagnostics.go translate before parsing, so all downstream rules apply unchanged and no diagnostic needs remapping. .test.mdl files joined `make check-mdl`; `.fail.test.mdl` names one whose annotations are deliberately unusable.", "insight": "Two lessons. First: the reporter's diagnosis was precise, confident and wrong, and the fastest way to find that out was to run the pipeline rather than read it — dumping GenerateTestFlows' output and feeding it to visitor.Build took one throwaway test and settled in seconds what an hour of grepping had not. Their error message was real; the command that produced it was not the one they named. Second, the general shape: a tool that OWNS a file format must not hand that format to a parser for a different one. The VS Code extension binds MDL to `.mdl`, which `.test.mdl` matches, so every test file in the editor was a wall of squiggles — 9 of this repo's 10 test files reported errors, one of them 392, and nobody had noticed because nobody runs `mxcli check` on a test file. When adding a derived file format, check what the EXISTING tooling makes of it; the answer is rarely 'nothing'. Line-preserving padding is what makes the translation honest: render into a slice of the source's own length and place wrapper fragments only on lines the original spent on comments or separators, and a diagnostic's line:col is the author's without a mapping table to drift."}
+{"area": "cmd/mxcli", "date": "2026-09-16", "symptom": "mendixlabs/mxcli#1104: `mxcli test --attach` reported only 'build failed: The project cannot be deployed, because it contains errors.' on an injection failure, and afterwards EVERY later run of ANY test file failed the same way until a leftover document was found by hand.", "cause": "Two independent defects. (1) The parsed problems were in hand and discarded: runner_attach.go and LocalApp.Rebuild both built their error with `fmt.Errorf(\"build failed: %s\", build.Message)`, and Message is identical for every failing build. Attribution (build_attribution.go, BuildResult.ErrorSummary) existed but was wired only into the --local BOOT, so --attach and every --watch rebuild lost it. (2) Generated names are positional — MxTest.Test_test_1, _2, … from the test's index in its file — and every test file reuses them, while cleanup dropped only the CURRENT suite's names. A run with fewer tests than the last one therefore left the surplus behind, and under --attach the MxTest module always pre-exists (the dev loop installed it) so the whole-module drop never fires.", "file": "cmd/mxcli/testrunner/cleanup_leftovers.go", "fix": "buildFailure()/resultsForBuildFailure() shared by both runners; cleanup keys on what the project HOLDS (SHOW MICROFLOWS IN MxTest, filtered by the generated prefix) with the suite only as a fallback; reportCleanup names every surviving document and prints its DROP.", "insight": "Measured, not reasoned: planted one bad MxTest.Test_test_2 in a project, ran a known-good one-test suite, and watched it fail and leave the leftover in place — then after the fix watched run 1 fail and CLEAN, and run 2 pass. A self-healing sequence is the control that 'cleanup works' cannot be argued into. The general rule for generated artefacts: derive what to remove from what EXISTS, never from what you intended to create. Keying on the suite was wrong in both directions at once — it missed leftovers AND issued DROPs for flows a part-way injection never created, and those failures made cleanup report 'the project has been left modified' for a project it had just cleaned, which is a false alarm that sends the reader hunting for damage. Positional names (index-in-file) guarantee collisions across files and are worth avoiding, but as long as they exist the prefix is the only safe key. Also worth pinning: once BuildFailedError.Error() renders the errors, a hint that repeats them prints everything twice — assert on the message the READER sees, not on the hint in isolation."}