Skip to content

Commit cb7cfa8

Browse files
authored
Merge pull request #1177 from ako/main
Sync ako/mxcli: storage-GUID data loss and its write guard, MOVE/RENAME reference sweeps
2 parents 57259e8 + 881aa7a commit cb7cfa8

74 files changed

Lines changed: 5496 additions & 97 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/lint-rules/conv010_act_microflow_content.star‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
# - ShowMessageAction (show message)
99
# - DownloadFileAction (download file)
1010
# - MicroflowCallAction (call sub-microflow for logic delegation)
11+
# - NanoflowCallAction (the same delegation from an ACT_ NANOFLOW)
1112
#
1213
# Business logic should be delegated to SUB_ microflows.
1314
# Requires FULL catalog (REFRESH CATALOG FULL).
@@ -37,6 +38,18 @@ ALLOWED_ACTIONS = (
3738
"ShowMessageAction",
3839
"DownloadFileAction",
3940
"MicroflowCallAction",
41+
# An ACT_ NANOFLOW delegates with a nanoflow call, not a microflow call.
42+
# microflows() yields nanoflows too (the catalog's `microflows` table carries
43+
# a MicroflowType column), so CONV010 lints them — and without this entry it
44+
# flagged the very delegation it demands: an ACT_ nanoflow could satisfy the
45+
# rule in no way at all. Reported from a real project, which patched its own
46+
# copy of the rule and asked for it upstream (ako/mxcli#644).
47+
#
48+
# This is the third time this allowlist has been short. It has held the wrong
49+
# vocabulary (storage names, matching nothing) and been missing an activity a
50+
# permitted one necessarily creates (ExclusiveMerge). The pattern is the same
51+
# each time: a rule that cannot be satisfied reads as the code being wrong.
52+
"NanoflowCallAction",
4053
# Storage names — belt and braces; see the note above.
4154
"ShowFormAction",
4255
"CloseFormAction",

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,4 @@
125125
{"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"]}
126126
{"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"]}
127127
{"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"]}
128+
{"area": "cmd/mxcli/test", "date": "2026-09-23", "symptom": "Windows: `mxcli test tests/ -p MyApp.mpr --local` fails with `local runtime: starting mxbuild serve: mxbuild --serve did not become ready` after caching a Linux ELF in %USERPROFILE%\\.mxcli\\mxbuild, and there is \"no flag, environment variable, or mechanism to redirect mxcli to the Windows mxbuild.exe already present in the Studio Pro installation\"", "cause": "Two layers. The platform part (downloading/exec'ing the Linux binary, serving from the cache instead of the resolved binary) was already fixed by #916 and #1122, both after the reporter's v0.21.0. What remained on main: `test` never registered `--mxbuild-path` — `run` gained it in #1125, but `test --local` boots through the same `ResolveMxBuildForLocal` and prints the same 'pass --mxbuild-path' guidance while answering `unknown flag`. `RunOptions` had no field and `localAppOptions` never set `LocalAppOptions.MxBuildPath`, though StartLocalApp honoured it. No env override existed anywhere", "file": "`cmd/mxcli/main.go` + `cmd_test_run.go` (flag), `testrunner/runner.go` + `localapp_options.go` (plumbing), `docker/mxbuild_platform.go` (`MxBuildPathEnv`, read in `resolveMxBuildForLocalOn` after the flag)", "insight": "**The guard for #1125 asserted its invariant against one command** — `TestErrorGuidanceNamesAFlagThatExists` checked only `runCmd`, while the guidance it polices is emitted by a resolver two commands share. When a test pins 'the advertised flag exists', enumerate the callers of the code that ADVERTISES it, not the command the report named; it now iterates `run` and `test`. **Before fixing a platform report, date it against the fixes**: the reporter's first two suggestions ('download platform-correct binary', 'auto-discover Studio Pro') were already on main, and re-implementing them would have been churn — only the override was missing. Put the env var in the resolver, not the CLI, so `run --local` and `test --local` both get it from one line; a flag-level env read would have been one more per-command copy to drift. Control: the env test runs as goos=windows with an unmatched version, so without the override it fails fast with the 'Linux binary cannot run natively on windows' refusal instead of hitting the CDN; removing only the `MxBuildPath:` line in localAppOptions fails the plumbing test for both runners. **Unverified**: no Windows host; code-level with OS-injected tests", "refs": ["mendixlabs/mxcli#1086", "mendixlabs/mxcli#1125", "mendixlabs/mxcli#916", "mendixlabs/mxcli#1122"]}

0 commit comments

Comments
 (0)