Sync ako/mxcli: widget bindings and action slots, nanoflow expression checks, diag invocation accuracy

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

@@ -122,3 +122,6 @@
 {"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"]}
+{"area":"cmd/mxcli","date":"2026-09-23","symptom":"`mxcli test`, `mxcli check`, `mxcli exec` with no arguments wrote 0 session_start records (`mxcli check /nonexistent.mdl` wrote 1), so a run that failed cobra's Args validation was invisible to `diag loop-report`.","cause":"#617 put diaglog.Init in the root's PersistentPreRun and described that as 'before argument validation'. Cobra's execute() runs ValidateArgs, and answers --help/--version, BEFORE any hook, so arity failures returned before the session opened. The same move also meant the root's -c and REPL sessions were recorded as mode \"mxcli\" instead of \"batch\"/\"repl\", because PreRun reached the singleton first.","file":"cmd/mxcli/session_start.go","insight":"A cobra hook is not 'before anything': execute() order is ParseFlags → help/version → ValidateArgs → PersistentPreRun, so anything that must see every invocation belongs in main() before Execute, keyed on rootCmd.Find(os.Args[1:]), which needs no parsed command. Moving the open that early moves the close problem with it: --help/--version return nil without running PersistentPostRun, so a close left there turns every help lookup into an 'unclosed' (failed) run. That was the false failure #617 had already paid for once. Close in main() after a nil Execute instead. When a singleton is opened earlier, whichever caller reaches it first picks the mode, so resolve the mode the later callers would have passed (batch/repl) at the early call site. The regression test runs the real main() in a re-executed test binary with MXCLI_LOG_DIR. It is the only layer that sees os.Exit paths, and both controls fail at the right place: stubbing startSession gives 0/0/0 records with the /nonexistent.mdl control still passing, and moving the close back to PostRun gives 0 session_end records for --help/--version.","refs":["ako/mxcli#633","ako/mxcli#617"]}
+{"area": "cmd/mxcli/diag", "date": "2026-09-23", "symptom": "`mxcli diag loop-report` showed all 5 `test` runs as 'did not close' although every test passed, and inflated the `-c` (111) and `exec` (180) counts in the same log. Reported as 'the command apparently skips the summary record on success too' \u2014 which is not what happens: `test` returns normally, PersistentPostRun fires, and the session_end IS written.", "cause": "mxcli runs mxcli. Measured from a real `mxcli test` with MXCLI_LOG_DIR pointed at a scratch dir: one parent session_start (pid 1071) followed by THREE child session_starts \u2014 `-c DESCRIBE SETTINGS`, `-c SHOW MODULES`, and an `exec` of the generated runner \u2014 before a single test executes. `new`, `eval`, `tui` and the LSP self-spawn the same way (six os.Executable() sites). buildInvocations segmented on 'next session_end OR next session_start, whichever comes first', so a child's start closed the parent's invocation and the parent's own end landed on whatever was open by then. session_end carried no pid, so pairing by process was impossible.", "file": "`mdl/diaglog/diaglog.go` (pid on session_end; parentPIDEnv marker set once in Init and inherited by every child), `cmd/mxcli/diag_loop_report.go` (buildInvocations pairs by pid with the positional rule as fallback; spawned runs excluded from the table and wall time, counted on their own line), tests `cmd/mxcli/diag_loop_report_test.go`", "insight": "The segmentation rule documented itself as exact 'for sequential invocations, which is what an agent loop produces' \u2014 and the thing that breaks that assumption is the tool itself, not concurrency by the user. When a tool can invoke itself, EVERY per-process measurement over it needs a parent link, not just a pid: a pid alone fixes the pairing but still counts three phantom agent calls per test run. The marker belongs on the ENVIRONMENT, not at each spawn site: exec.Command inherits the parent's environment (explicitly via os.Environ(), implicitly when Cmd.Env is nil), so one os.Setenv in Init covers all six self-spawn sites and any added later \u2014 six edits that would each have to be remembered become zero. Second-order trap: spawned runs must be excluded from WALL TIME too, not just the count, because a child's seconds are already inside its parent's; the test asserts 10s for a parent with three 1s children, and the reverted code says 3. Prove-by-revert done on the measured record shape: the positional rule gives Invocations=4 (want 1), Unclosed=1 for a parent whose tests all passed, Wall=3 (want 10).", "refs": ["ako/mxcli#617", "ako/mxcli#629"]}
+{"area": "cmd/mxcli/syntax", "date": "2026-09-23", "symptom": "`mxcli syntax page datasource` documented `DataSource: MICROFLOW Module.MF($P)`. That form is a parse error: `dataview dv (datasource: microflow M.DS_X($State))` gives 'line 2:15 no viable alternative at input datasource'. Only the NAMED form `M.DS_X(State: $State)` parses. Hit in a real build, diagnosed from the error rather than the doc.", "cause": "The syntax entry was written from the intended shape rather than from something that had been run through the parser. Nothing checks it: the Syntax and Example fields are free text.", "file": "`cmd/mxcli/syntax/features_page.go` (page.datasource entry now shows `MICROFLOW Module.MF(Param: $P)` and states that the positional form is a parse error)", "insight": "CLAUDE.md deliberately points at `mxcli syntax` instead of restating syntax, so that it cannot go stale \u2014 which makes a wrong entry there worse than a wrong entry in prose, because it is the thing consulted INSTEAD of checking. The cost lands in the agent loop: read it, write it, fail to parse, diagnose, retry. Measured before reaching for the systemic guard: 42 of 164 syntax examples fail `mxcli check` today, but the large majority are fragments by design (a microflow body like `IF \u2026`, a widget snippet like `DATAGRID \u2026`, an OQL fragment) and are legitimately not standalone top-level MDL \u2014 so a blanket 'every example must parse' test would be mostly noise, and making it useful needs a way to mark which examples are standalone. Measuring that first is what stopped a plausible-sounding guard from being built wrong.", "refs": ["ako/mxcli#630"]}