Skip to content

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

Merged
ako merged 30 commits into
mendixlabs:mainfrom
ako:main
Sep 23, 2026
Merged

ako merged 30 commits into
mendixlabs:mainfrom
ako:main

Conversation

@ako

@ako ako commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

12 commits from the fork's main, on top of the last sync (c127385). Grouped by theme.

Page and widget authoring

  • Entity-type JavaScript action parameters (d85c65a, fixes CALL JAVASCRIPT ACTION with entity-type parameter produces wrong BSON type (BasicCodeActionParameterValue instead of EntityTypeCodeActionParameterValue) #1137) — call javascript action wrote BasicCodeActionParameterValue{Argument} where Studio Pro stores EntityTypeCodeActionParameterValue{Entity}, leaving the Studio Pro entity picker empty and failing the build with CE0115. The builder never looked the action up; it hardcoded Basic for every parameter, with the bug stated as a comment. It now resolves the signature and promotes parameters typed to the generic entity <> slot, mirroring the Java-action builder. Measured on 11.6.6: 0 errors fixed, CE0115 faulty, with the two BSON shapes the report quotes on disk. DESCRIBE renders the broken and the correct nanoflow as identical MDL, so the regression test asserts on the $Type rather than on a round trip.
  • ALTER PAGE can retarget a named action slot (328828c, ALTER PAGE SET cannot address named pluggable-widget action slots (createFileAction etc.) — follow-up to #956 #995) — set 'createFileAction' = microflow M.F on fileUploader1 failed to parse. Named action slots were made writable on CREATE, but alterPageAssignment only accepted an action expression for the key Action, so retargeting one slot meant REPLACEing the whole widget and restating every other property. The mutator refuses a key whose PropertyType is not Action-typed and names the widget's slots, since every WidgetValue carries an Action field — field presence would accept an Integer property and change nothing. Measured on 11.12.1: retargeting DataGrid 2's onSelectionChange keeps PageSize and passes mx check at 0 errors.
  • Action button captionparams (edfad3f, closes captionparams on an action button passes check and is never written ako/mxcli#632) — accepted by check and not written. Two silent drops: the button builder had its own copy of the template-parameter resolver, which wrote a bare attribute as a literal, and DESCRIBE emitted ContentParams:, which that builder never read — so describe → exec dropped them all and mx check reported CE0720. The shared resolver also brings SourceVariable, association paths and per-parameter format blocks to button captions, and MDL-WIDGET04's orphan-placeholder check now covers them.
  • A gallery's template block is a slot, not a duplicate name (f275a58, DESCRIBE PAGE output is rejected by mxcli's own check --references (duplicate derived widget names) #978, third symptom) — describe page on a page with several galleries emitted MDL that mxcli's own check --references rejected for a duplicate widget name template1. template and filter are child slots of the gallery's definition: the block's name is discarded on write, which is why DESCRIBE synthesises one per gallery. Measured on 11.12.2: a block authored with a name appears in 0 stored files, and four galleries each holding template template1 pass mx check at 0 errors. Widgets inside a slot still count, and a standalone template — a real container that does store its name — is still checked.
  • A list view that renders its inputs read-only (0dc5c0f, 1baabaf, List views are written with Editable: false, so every input inside one renders disabled ako/mxcli#631) — a listview that does not say editable: true is written non-editable, and its read-only context wins over editable: Always on the inputs inside it, including inside a nested data view. Valid BSON, clean mx check, clean build; only the running app shows it. The default is kept, since it is Mendix's own — instead MDL-WIDGET31 warns. Editability is read the way the builder reads it, so a quoted editable: 'true' (written false) warns too. Pinned against Studio Pro-authored content at 11.14.0: 30 of 32 list views are stored non-editable and none of those holds an input; the one with inputs was set editable.

check catches more before the build

DESCRIBE

Workflows

diag loop-report accuracy

Two defects that made the report describe a different set of runs than the agent actually made.

  • mxcli runs mxcli (63588cd, closes diag loop-report: mxcli's own child processes are counted as loop calls, and their parent reads as unclosed ako/mxcli#629) — a project reported all 5 test runs as "did not close" while every test passed. test closes its session correctly; the report gave the record to the wrong invocation. Measured: three child processes spawn before the first test executes, and new, eval, tui and the LSP self-spawn the same way. Segmentation closed a parent on a child's start, and the parent's own end — arriving last — landed on whatever was open by then. Two consequences, both fixed: the parent read as a non-zero exit, and the children were counted as calls the agent made, inflating that log's -c and exec counts. Pairing is now by pid with the positional rule kept as a fallback, and the marker rides the environment rather than each spawn site, so it covers all six and any added later. Proven by revert on the measured record shape, with three controls.
  • A run that fails argument validation (e7862f5, closes A command called with the wrong number of arguments writes no session record at all ako/mxcli#633) — test, check and exec with no arguments wrote no session record at all, because cobra runs argument validation and answers --help/--version before any hook. The session now opens in main(). Two things had to move with it: the close, since --help returns without running the post-hook and would otherwise leave every help lookup reading as a failed run; and the root command's mode, since whoever reaches the singleton first names the session and -c and the REPL were being recorded as neither.

Syntax reference

  • page.datasource documented a form that cannot parse (45e0ae8, closes syntax page.datasource documents a microflow-argument form that is a parse error ako/mxcli#630) — it printed a microflow datasource with a positional argument; arguments must be named. Found from the parse error rather than from the doc. CLAUDE.md points at mxcli syntax as authoritative instead of restating syntax, precisely so it cannot go stale — which makes a wrong entry the thing consulted instead of checking, and puts the cost in the agent loop. The systemic guard is deliberately not built here, and the reason is measured: 42 of 164 examples fail check, but the large majority are fragments by design, so a blanket "every example must parse" test would be mostly noise until there is a way to mark which examples stand alone.

claude and others added 30 commits September 22, 2026 10:55
…ionParameterValue

`call javascript action` with an entity-type parameter wrote
Microflows$BasicCodeActionParameterValue{Argument: "Mod.Entity"} where
Studio Pro stores
Microflows$EntityTypeCodeActionParameterValue{Entity: "Mod.Entity"}.
The Basic shape leaves the Studio Pro entity picker empty and fails the
build with CE0115.

addCallJavaScriptActionAction never looked the action up; it hardcoded
Basic for every parameter, with the bug stated as a comment. It now
resolves the signature through ReadJavaScriptActionByName (already on the
backend interface) and promotes parameters typed
CodeActions$EntityTypeParameterType — the generic `entity <>` slot —
mirroring the Java-action builder, which has drawn this distinction since
#656. A parameter typed to a concrete entity keeps the Basic
shape, and without a backend the prior behaviour stands.

The read, write and DESCRIBE paths already handled the correct type, so
the change is confined to the builder.

Measured on mxbuild 11.6.6 against testdata/expr-checker/minimal.mpr
(which ships NanoflowCommons), two builds of the same script:

  fixed  -> "The app contains: 0 errors."
  faulty -> CE0115 "The arguments that are passed to JavaScript action
            'NanoflowCommons.RefreshEntity' do not match the expected
            parameters and need to be refreshed."

and the BSON on disk carries $Type
Microflows$EntityTypeCodeActionParameterValue / "Entity" fixed, versus
Microflows$BasicCodeActionParameterValue / "Argument" faulty — the two
snippets the report quotes.

Control: with the type check stubbed out, the new executor test fails with
the reported symptom ("value = *microflows.BasicCodeActionParameterValue,
want *EntityTypeCodeActionParameterValue") while both its controls still
pass. DESCRIBE renders the broken and the correct nanoflow as identical
MDL, so the regression test asserts on the BSON $Type rather than on a
round trip.

Fixes mendixlabs#1137

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ub4kEQmBAN1zk18fjNQppR
`alter page M.P { set 'createFileAction' = microflow M.F on fileUploader1; }`
failed to parse (mendixlabs#995):

  line 2:37 extraneous input 'MyModule' expecting {DROP, ADD, SET, INSERT, REPLACE, '}'}

mendixlabs#956 made named action slots writable on CREATE PAGE, but alterPageAssignment
only accepted an action expression for the key `Action`, so retargeting one
slot meant REPLACEing the whole widget and restating every other property.

- grammar: `'<key>' = actionExprV3` and `<key> = actionExprV3`, ahead of the
  scalar alternatives as on CREATE
- executor: an action keyed anything but `Action` routes to the new
  PageMutator.SetWidgetNamedAction; refused on a column or page-level target,
  where the setters would stringify it
- pagemutator: writes Value.Action (the field widgetobj.Builder.SetAction
  writes) and refuses a key whose PropertyType is not Action-typed, naming the
  widget's action slots. Every WidgetValue carries an Action field, so field
  presence would accept an Integer property and change nothing
- MCP backend refuses, as it does for SetWidgetAction

Measured on 11.12.1: retargeting DataGrid 2's onSelectionChange via ALTER
keeps PageSize and passes `mx check` with 0 errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YVdoySKj31N5PZmjrAGeEh
)

DESCRIBE JAVA ACTION printed `ContextObject: entity <>` for a parameter
declared `entity <pEntity> not null`. The stored type holds only a BY_ID
pointer to the TypeParameter; javaActionFromGen carried the ID across but
never resolved the name, which is all the describer prints. The JavaScript
action reader already had this pass. Bare type-parameter references in
parameters and the return type are resolved the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011VrrQgJ6aXdEJ8Db1fQx5G
…abs#1033)

`currentDeviceType()` in a nanoflow passed `mxcli check` and `mxcli exec`,
then failed the build with CE0117 "Error(s) in expression." The mendixlabs#828 fix
wired MDL044 to CREATE MICROFLOW only. Separately, `log` message, node and
template expressions were never checked for MDL044 in either flow type, and
that is where the reported repro puts the call.

- checkStmtExprFunctions: the single list of MDL044 expression sites, now
  including log statements, shared by walkBody and the nanoflow walker
- ValidateNanoflow runs MDL044 only. The full microflow rule set would be
  a false positive here: MDL057 refuses `synchronize`, which nanoflows allow
- wired into check (validate_program), the LSP, and exec
  (validateNanoflowRules in buildNanoflowFromStmt, same allowlist as
  microflows)

Verified on mxbuild 11.13.0: the pre-fix binary writes the repro and the
build reports CE0117; the fixed binary refuses it in check and exec. The
new sites' positive forms (toUpperCase in a log template,
toString([%CurrentDateTime%])) build at 0 errors. The report's workaround
`[%CurrentDeviceType%]` is itself CE0117 in both flow types; the skill
notes that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M1J643ZJw61bNaUSDbZkBj
…s#1024)

mendixlabs#1024 reported that `jump to A;` in a boundary-event body
was written as a jump NAMED A:

  [CE0495] "Duplicate name 'A'." at User task 'A', Jump 'A'
  [CE6680] "The 'Target' property is required." at Jump 'A'

This has the same root cause as mendixlabs#1005, and 825873d already fixed it. The
report came from v0.20.0, which predates that commit. Measured on a
blank 11.14.0 project with native mx check:

  825873d^  2 errors (CE0495 + CE6680, verbatim)
  825873d   0 errors
  main       0 errors

The mendixlabs#1005 tests only put jumps in outcome flows, so this adds
coverage for the boundary-event body (user task and wait for
notification), plus a bug-test script and a finding.

Controls: the unchanged test fails with the reported duplicate-name and
self-target symptoms against 825873d^. On main it fails the same way
only when BOTH mendixlabs#1005 guards are stubbed (jump named after its target, and
jumps deduplicated last). Either guard alone keeps it green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012owAFCYmF2aAUY2S5phmGQ
… duplicate

`describe page` on a page with several galleries emitted MDL that mxcli's own
`check --references` rejected. mendixlabs#978, third symptom:

  - duplicate widget name 'template1' (used 4 times) — Mendix requires unique
    widget names per page (CE0495)

The row/col names in the same report were fixed by 587ed31; this one was not
covered. `template` and `filter` inside a gallery are child slots of its
definition. applyChildSlots stores only the block's children in the slot
property and discards the block's name, which is why DESCRIBE synthesises
`template1` for every gallery. Measured on 11.12.2: a block authored as
`template contentZebra` appears in 0 stored files and describes back as
`template1`; four galleries each holding `template template1` pass `mx check`
with 0 errors.

The check already exempted the enclosing widget's object-list containers; it
now exempts its child-slot containers too (definition and every mode), plus the
`container <slotName>` spelling that applyChildSlots routes by name. Widgets
inside a slot still count, and a standalone `template` (a Forms$DivContainer,
which does store its name) is still checked.

Control: with the fix reverted, the new test fails with the reported line,
`duplicate widget name 'template1' (used 4 times)`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qf5Bk8ZMQkwCNEheapPzq4
…ted them

A second test project reported all 5 `test` runs as "did not close" while
every test passed, reading it as "the command skips the summary record on
success too". It does not: `test` returns normally, PersistentPostRun fires,
and the session_end is written. The report gave it to the wrong invocation.

**mxcli runs mxcli.** Measured from a real run:

  session_start  pid=1071   test t.test.mdl --skip-build -p …
  session_start  pid=1079   -p …/minimal.mpr -c DESCRIBE SETTINGS
  session_end    pid=None
  session_start  pid=1086   -p …/minimal.mpr -c SHOW MODULES
  session_end    pid=None
  session_start  pid=1093   exec /tmp/mxtest-runner-….mdl -p …

Three child processes before the first test executes; `new`, `eval`, `tui`
and the LSP self-spawn the same way. buildInvocations segmented on "next
session_end OR next session_start, whichever comes first", so a child's start
closed the parent and the parent's own end — arriving last — landed on
whatever was open by then. session_end carried no pid, so pairing by process
was impossible. The rule called itself exact "for sequential invocations,
which is what an agent loop produces"; the tool itself is what breaks that.

Two consequences, both fixed: the parent read as a non-zero exit, and the
three children were counted as calls the agent made — inflating that log's
`-c` (111) and `exec` (180) counts.

session_end now carries its pid and pairing is by process, with the positional
rule kept as the fallback so older logs still read correctly. session_start
carries parent_pid, and the marker rides the ENVIRONMENT rather than each
spawn site: a child inherits it through exec.Command, so one os.Setenv in Init
covers all six self-spawn sites and any added later without touching them.
A spawned run is excluded from the table and from wall time — its seconds are
already inside its parent's — and reported on its own line.

Proven by revert on the measured record shape: the positional rule gives
Invocations=4 (want 1), Unclosed=1 for a parent whose tests all passed, and
Wall=3 instead of 10. Three controls: an old-format log with no pids still
pairs positionally, a top-level run carrying a pid is not treated as spawned,
and an orphan session_end from outside the window is dropped rather than
applied to whichever invocation is open.

Closes #629

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MgcYSQrLLbUcnAMaCHpyqQ
…t cannot parse

`mxcli syntax page datasource` printed `MICROFLOW Module.MF($P)`. Measured:

  dataview dv (datasource: microflow M.DS_X($State))    -> line 2:15 no viable
                                                           alternative
  dataview dv (datasource: microflow M.DS_X(State: $State)) -> Syntax OK

Arguments must be named. Reported independently from a test project, found
from the parse error rather than from the doc.

CLAUDE.md points at `mxcli syntax` as authoritative instead of restating
syntax, precisely so it cannot go stale — which makes a wrong entry there the
thing consulted INSTEAD of checking, and puts the cost in the agent loop:
read it, write it, fail to parse, diagnose, retry.

The systemic guard is deliberately not built here. Measured first: 42 of 164
syntax examples fail `mxcli check`, but the large majority are fragments by
design — a microflow body, a widget snippet, an OQL fragment — which are
legitimately not standalone MDL. A blanket "every example must parse" test
would be mostly noise; making it useful needs a way to mark which examples
stand alone. Left in #630 with that measurement.

Closes #630

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MgcYSQrLLbUcnAMaCHpyqQ
fix(executor): entity-type JS action parameters get EntityTypeCodeActionParameterValue
fix(alter-page): SET can address a pluggable widget's named action slot
fix(describe): resolve java action type-parameter names (mendixlabs#1034)
fix(check): a gallery's template/filter block is a slot, not a CE0495 duplicate
A listview that does not say `editable: true` is written Editable false,
and its read-only context wins over `editable: Always` on the inputs inside
it, including inside a nested data view. Valid BSON, clean mx check, clean
build; only the running app shows it.

The default is kept: false is Mendix's own (mendixmodelsdk 4.115.0,
Pages$ListView.editable defaults to false and _initializeDefaultProperties
does not set it). Instead, MDL-WIDGET31 warns on a list view that will be
written non-editable while it holds an input not marked `editable: Never`.
Editability is read with GetBoolProp, as buildListViewV3 does, so a quoted
`editable: 'true'` (written false) warns too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01266huGoXavWeZvPq8BV7LT
Closes #633. `mxcli test`, `check` and `exec` with no arguments wrote no
session_start at all, so `diag loop-report` never saw them:

    probe                        before  after
    mxcli test                     0       1
    mxcli check                    0       1
    mxcli exec                     0       1
    mxcli check /nonexistent.mdl   1       1   (control)

#617 opened the session from PersistentPreRun, but cobra runs ValidateArgs
(and answers --help/--version) before any hook. The session now opens in
main() before Execute, named from rootCmd.Find(os.Args[1:]); diag stays
excluded.

Two things had to move with it:

- The close. --help and --version return nil without running
  PersistentPostRun, so with the earlier open every help lookup would have
  been left unclosed, which the report reads as a failed run. main() now
  closes after a nil Execute; os.Exit paths still leave no session_end.
- The root's mode. Whoever reaches the singleton first names the session,
  and PreRun had been recording -c and the REPL as "mxcli" instead of
  "batch"/"repl", the names invocationVerb falls back to. startSession
  resolves those, plus "help"/"version" for the root flags.

The tests re-run the test binary as main(), the only layer that sees the
os.Exit path. Controls: stubbing startSession brings back 0/0/0 while the
/nonexistent.mdl control still passes. Moving the close back to PostRun
gives 0 session_end for check --help, --help and --version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FM9DSqkPmhW5KcLtQLgsXn
…scribe

`captionparams` on an action button was accepted by `mxcli check` but not
written (#632). Two silent drops:

- buildButtonV3 had its own copy of the template-parameter resolver that
  wrote a bare attribute (`[{1} = Title]`) as the literal 'Title'. It now
  uses the shared buildClientTemplateParams, like dynamictext and grid
  columns, which also brings SourceVariable, association paths and
  per-parameter format blocks to button captions.
- DESCRIBE printed a button's parameters as `ContentParams:`, which the
  button builder never read, so describe -> exec dropped them all and
  mx check reported CE0720. DESCRIBE now emits `CaptionParams:`, and the
  builder still reads `ContentParams:` for scripts dumped before this fix.

MDL-WIDGET04's orphan-placeholder check now covers action/link button
captions, so a `{1}` with no parameter is refused at check time instead
of failing the build with CE0720.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kS7YrCUmW7uFQ1PK6zjkf
)

ako/TestApp (Mendix 11.14.0), authored in Studio Pro: 30 of its 32 list
views are stored Editable false and none of those holds an input; the one
list view with inputs (Pages.EditableLIstView) was set to Editable true.
Its describe output checks clean, deleting `Editable: true` from it fires
MDL-WIDGET31, and describe -> exec preserves the property.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01266huGoXavWeZvPq8BV7LT
test(workflow): pin `jump to` inside a boundary-event body (mendixlabs#1024)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M1J643ZJw61bNaUSDbZkBj
fix(check): run MDL044 over nanoflow bodies and log messages (mendixlabs#1033)
Two defects a second test project turned up, and three more filed with their measurements
fix: action button captionparams bind a bare attribute and survive describe
fix: a command called with the wrong number of arguments writes no session record
fix(check): warn when a list view renders its inputs read-only (MDL-WIDGET31)
@github-actions

Copy link
Copy Markdown

AI Code Review

Let me analyze this pull request against the review checklist.

First, I'll look at the PR description to understand what it's about:

The PR is titled "Sync ako/mxcli: widget bindings and action slots, nanoflow expression checks, diag invocation accuracy"

Looking at the body, it contains several fixes:

Page and widget authoring

check catches more before the build

DESCRIBE

Workflows

diag loop-report accuracy

Syntax reference

Now, I need to review this against the checklist.

Let me go through the checklist items:

Overlap & duplication

  • Check proposals list above for existing proposals covering the same functionality
  • Check if the diff introduces code that duplicates existing implementations (look for similar function names, command names, types)
  • Check doctype test from doing code that duplicates existing implementations

Looking at the diff, I see:

  • New test files for various fixes (e.g., mutator_named_action_test.go, button_caption_params_632_test.go, etc.)
  • Many new test files in mdl-examples/bug-tests/ for reproducing the bugs
  • Changes to existing files to fix the issues

I don't see obvious duplication - these appear to be legitimate fixes for reported issues.

MDL syntax design

The PR doesn't seem to add new MDL syntax - it's mostly fixing existing functionality. Let me check:

  • For the ALTER PAGE named action slot: This is extending existing ALTER PAGE syntax to support named action slots, which was already supported in CREATE PAGE. This follows the pattern.
  • For the JavaScript action entity-type parameter: This is fixing how existing CALL JAVASCRIPT ACTION statements are processed, not changing the syntax.
  • For the button captionparams: This is fixing how existing CAPTIONPARAMS syntax is handled.
  • For the gallery template block: This is fixing how existing syntax is interpreted, not changing the syntax.
  • For the listview editable inputs: This is adding a warning for existing syntax.
  • For the nanoflow MDL044: This is extending an existing check to nanoflows.
  • For the DESCRIBE Java action: This is fixing how existing DESCRIBE JAVA ACTION output is generated.
  • For the workflow jump to: This is fixing how existing JUMP TO syntax is processed in boundary events.
  • For the diag loop-report: This is fixing internal diagnostics, not MDL syntax.
  • For the syntax reference: This is fixing documentation, not MDL syntax.

So no new MDL syntax is being added - just fixing existing functionality.

Full-stack consistency

Since no new MDL syntax is being added, this section doesn't apply. The fixes are to existing features, and they appear to be properly wired through the stack where needed (grammar changes, visitor changes, executor changes, etc. as appropriate for each fix).

Test coverage

  • New packages should have test files: I don't see entirely new packages, but there are many new test files for the fixes.
  • New executor commands should have MDL examples in mdl-examples/doctype-tests/: Not applicable as no new commands.
  • Integration paths should be tested: Yes, there are integration tests (e.g., the diag_loop_report_test.go has integration-style tests).
  • No time.Sleep for synchronization: I don't see any time.Sleep calls in the diff.

Security & robustness

I don't see obvious security issues. The changes seem to be focused on correctness.

Scope & atomicity

Each commit appears to do one thing:

  • d85c65a: Entity-type JavaScript action parameters
  • 328828c: ALTER PAGE can retarget a named action slot
  • edfad3f: Action button captionparams
  • f275a58: A gallery's template block is a slot
  • 0dc5c0f, 1baabaf: A list view that renders its inputs read-only
  • f9a781f: MDL044 over nanoflow bodies and log messages
  • 7c158bc: Java action type-parameter names
  • cb52ae7: jump to inside a boundary-event body
  • 63588cd: mxcli runs mxcli (diag loop-report)
  • e7862f5: A run that fails argument validation (diag loop-report)
  • 45e0ae8: page.datasource documented a form that cannot parse

These seem appropriately scoped.

Code quality

I don't see obvious code quality issues. The changes seem consistent with the existing codebase.

Bugs & correctness

The PR is fixing bugs, so this is about whether the fixes are correct.

Let me check a few specific fixes against the checklist requirements:

  1. Entity-type JavaScript action parameters (CALL JAVASCRIPT ACTION with entity-type parameter produces wrong BSON type (BasicCodeActionParameterValue instead of EntityTypeCodeActionParameterValue) #1137):

    • The fix resolves the signature and promotes parameters typed to the generic entity <> slot
    • Added tests: javascript-action-1137-entity-type-parameter.mdl
    • Changes: cmd_microflows_builder_calls.go, cmd_microflows_builder_js_action_test.go, microflow_action_test.go
    • This appears correct - it's fixing the builder to properly handle entity-type parameters
  2. ALTER PAGE can retarget a named action slot (ALTER PAGE SET cannot address named pluggable-widget action slots (createFileAction etc.) — follow-up to #956 #995):

    • Fixes the issue where set 'createFileAction' = microflow M.F on fileUploader1 failed to parse
    • Added tests: 995-alter-page-set-named-action-slot.mdl, alter_set_named_action_test.go, visitor_alter_page_named_action_test.go, mutator_named_action_test.go
    • Changes: cmd_pages_builder_v3_widgets.go, cmd_alter_page.go, visitor_alter_page_named_action_test.go, mutator.go, alter_set_named_action_test.go
    • This adds support for named action slots in ALTER PAGE, which was missing
  3. Action button captionparams (fix: ALTER STYLING writes design properties on pages and snippets (#631) #632):

    • Fixes silent drops where captionparams were accepted by check but not written
    • Added tests: 632-button-captionparams-bare-attribute.mdl, button_caption_params_632_test.go
    • Changes: cmd_pages_builder_v3_widgets.go, cmd_pages_describe_output.go, button_caption_params_632_test.go
    • This fixes both the writing and reading of captionparams
  4. Gallery's template block is a slot (DESCRIBE PAGE output is rejected by mxcli's own check --references (duplicate derived widget names) #978):

    • Fixes duplicate widget name issue when describing pages with galleries
    • Added tests: 978-gallery-slot-names-not-duplicates.mdl, validate_page_slot_containers_test.go
    • Changes: cmd_pages_builder_v3_widgets.go, cmd_pages_describe_output.go, validate_page_slot_containers_test.go
    • This correctly treats template/filter blocks as slots rather than widgets with stored names
  5. List view inputs not editable (ALTER STYLING cannot set design properties on widgets in pages and snippets #631):

    • Adds warning for list views that will render inputs read-only
    • Added tests: 631-listview-inputs-not-editable.mdl, validate_listview_editable_inputs_test.go, validate_listview_editable_inputs.go
    • Changes: validate_listview_editable_inputs.go, validate_listview_editable_inputs_test.go
    • This adds a diagnostic (MDL-WIDGET31) for the problematic pattern
  6. MDL044 over nanoflow bodies (MDL044 write barrier (currentDeviceType() validation) only runs for CREATE MICROFLOW, not CREATE NANOFLOW — the identical invalid expression passes check, is written, and fails the build with CE0117 #1033):

    • Extends MDL044 (unknown function check) to nanoflows
    • Added tests: 1033-nanoflow-log-expressions-ok.mdl, 1033-nanoflow-unknown-expression-function.fail.mdl, validate_nanoflow_test.go, validate_nanoflow.go
    • Changes: validate_nanoflow.go, validate_nanoflow_test.go, validate_microflow.go (modified to not run full set on nanoflows)
    • This correctly applies the expression validation to nanoflows
  7. Java action type-parameter names (DESCRIBE JAVA ACTION drops the type-parameter name (prints entity <> instead of entity <pEntity>) — the declaration does not round-trip #1034):

    • Fixes DESCRIBE JAVA ACTION to show type parameter names
    • Added tests: 1034-describe-java-action-type-parameter-name.mdl
    • Changes: java_read.go, java_read_test.go
    • This fixes the describer to resolve type parameter names
  8. Jump to in boundary-event body (JUMP TO <activity> inside a boundary-event body writes a Jump with no Target, named after its target — so an interrupting boundary event cannot be expressed correctly #1024):

    • Fixes jump to inside boundary-event bodies
    • Added tests: 1024-boundary-event-jump-to.mdl, issue1024_boundary_jump_test.go
    • Changes: issue1024_boundary_jump_test.go
    • This adds test coverage for the boundary-event case
  9. Diag loop-report accuracy (Add a no-Docker local run/test path (mxcli run --local) and document it in skills #629, DESCRIBE emits over-qualified grant member names that fail to re-parse (roundtrip break) #633):

    • Fixes issues with session tracking when mxcli spawns child processes
    • Added tests: diag_loop_report_test.go (many new tests), session_start.go, session_start_test.go
    • Changes: diag_loop_report.go, diag_loop_report_test.go, main.go, session_start.go, session_start_test.go, lsp_diagnostics.go
    • This moves session initialization to main() to catch all invocations, including those that fail argument validation
  10. Page.datasource documentation (Daily Digest: 2026-06-02 #630):

    • Fixes documentation to show correct syntax (named arguments required)
    • Changes: MDL_QUICK_REFERENCE.md
    • This corrects the documentation to match the actual syntax requirement

Now, let me check if there are any violations of the checklist:

Looking at the full-stack consistency requirement: For existing features being fixed, they should already be wired through the full pipeline. The fixes appear to be making sure the existing features work correctly through the pipeline.

For example:

  • The ALTER PAGE named action slot fix adds grammar rules (in MDLParser.g4), visitor handlers (in visitor_alter_page.go), and executor logic (in mutator.go and alter_set_named_action_test.go)
  • The JavaScript action entity-type parameter fix modifies the builder logic (in cmd_microflows_builder_calls.go) and adds tests
  • The button captionparams fix modifies both the builder (in cmd_pages_builder_v3_widgets.go) and the describer (in cmd_pages_describe_output.go)
  • The gallery template block fix modifies both the builder and describer
  • The listview inputs fix adds a validator (in validate_listview_editable_inputs.go)
  • The nanoflow MDL044 fix adds a validator for nanoflows (in validate_nanoflow.go)
  • The Java action type-parameter fix modifies the reader (in java_read.go)
  • The diag loop-report fixes modify the session initialization and reporting logic

These all appear to be properly wired where needed.

Let me check if there are any specific issues mentioned in the checklist that I should watch for:

  • "Check if the diff introduces code that duplicates existing implementations" - I don't see obvious duplication
  • "Check doctype test files above for existing test coverage of the feature area" - The PR adds test files for the bug tests, which is good
  • "Flag if the PR re-documents already-shipped features as new" - The PR is fixing bugs, not documenting features as new

One thing I should check: the PR mentions "DESCRIBE renders the broken and the correct nanoflow as identical MDL, so the regression test asserts on the $Type rather than on a round trip." for the JavaScript action fix. This is acceptable because DESCRIBE shouldn't be expected to show internal representation differences that don't affect the MDL roundtrip.

Let me look for any potential issues:

In the diag_loop_report.go changes, I see:

  • Added PID and ParentPID fields to logRecord
  • Modified buildInvocations to pair by PID
  • Added Spawned field to invocation
  • Modified analyzeLoop to exclude spawned runs from counts and add a separate Spawned count

This seems correct for fixing the issue where mxcli's own spawned processes were being counted as agent invocations.

In session_start.go, the session is now started in main() rather than PersistentPreRun to catch invocations that fail argument validation. This is a good fix for #633.

Let me check if there are any MDL syntax issues:

For the ALTER PAGE named action slot, the visitor test shows:

alter page MyModule.UploadPage {
  set 'createFileAction' = microflow MyModule.ACT_CreateFile on fileUploader1;
};

This follows the standard syntax: SET property = value on widget.

For the button captionparams, the test shows:

"CaptionParams": [{1} = Title]

This follows the property format with colon separators (though in the test it's shown as a map, the actual MDL would be CaptionParams: [{1} = Title]).

For the gallery template block, the issue was that DESCRIBE was synthesizing names like template1 for slots that don't store names, causing duplicate name errors. The fix correctly identifies these as slots rather than widgets.

I don't see any violations of the MDL syntax guidelines:

  • Uses standard CRUD verbs (CREATE/ALTER/DROP/SHOW/DESCRIBE)
  • Uses qualified names (Module.Element)
  • Property lists use (Key: value, ...) format
  • Colon for property definitions, AS for name mappings (I don't see AS being misused in the changes)
  • Keywords are full English words
  • Statements read as English
  • No keyword overloading apparent
  • Diff-friendly changes
  • Consistent with existing MDL patterns

One thing to double-check: in the visitor_alter_page_named_action_test.go, I see:

set 'createFileAction' = microflow MyModule.ACT_CreateFile on fileUploader1;

This uses quotes around the property name, which is correct for property keys that might be keywords or contain special characters. The test also shows bare identifiers working.

Now, let me check if there are any specific checklist items I might have missed:

Under "Full-stack consistency (for MDL features)":

  • Grammar rule added to MDLParser.g4 (and MDLLexer.g4 if new tokens)
  • AST node type added in mdl/ast/
  • Visitor handler in mdl/visitor/
  • Executor handler in mdl/executor/
  • LSP wiring (cmd/mxcli/lsp.go) if the feature adds formatting, diagnostics, or navigation
  • DESCRIBE roundtrip — if the feature creates artifacts, DESCRIBE should output re-executable MDL
  • VS Code extension (vscode-mdl/package.json) updated if new LSP capabilities added

For the ALTER PAGE named action slot fix:

  • Grammar: I don't see explicit changes to MDLParser.g4 in the diff, but the visitor test implies it parses correctly. Let me check if there are grammar changes...
    Looking at the diff, I don't see changes to mdl/grammar/MDLParser.g4 in the selected files, but the PR description mentions it's grouped by theme and the diff is SELECTED. The visitor test shows it parses, so the grammar must have been updated (possibly in the non-selected parts of the diff).
  • AST: Likely uses existing ActionV3 or similar AST nodes
  • Visitor: visitor_alter_page_named_action_test.go shows visitor handling
  • Executor: alter_set_named_action_test.go shows executor handling
  • LSP: Not mentioned, but this is about parsing/modifying existing functionality, not adding new LSP features
  • DESCRIBE: Not directly affected by this fix
  • VS Code extension: Not mentioned as needing update

For the JavaScript action entity-type parameter fix:

  • Grammar: No change needed (uses existing CALL JAVASCRIPT ACTION)
  • AST: No change needed
  • Visitor: No change needed
  • Executor: cmd_microflows_builder_js_action_test.go shows the fix in the builder
  • LSP: Not needed
  • DESCRIBE: Not directly affected
  • VS Code extension: Not needed

For the button captionparams fix:

  • Grammar: No change needed (uses existing syntax)
  • AST: No change needed
  • Visitor: No change needed
  • Executor: button_caption_params_632_test.go shows fixes in both builder and describer
  • LSP: Not needed
  • DESCRIBE: cmd_pages_describe_output.go is modified (this is the describer)
  • VS Code extension: Not mentioned as needing update

And so on for the other fixes.

Since the diff is SELECTED and we're only seeing 19 of 57 files, it's likely that the necessary grammar/AST/visitor changes are in the non-selected parts.

Let me check if there are any obvious issues with the changes I can see:

In session_start.go:

// startSession records the invocation before anything can reject it.
// `diag loop-report` reports "mxcli invocations: N", and burning calls on
// malformed invocations is exactly the agent failure it is opened to see
// (ako/mxcli#617). It runs from main() rather than PersistentPreRun because
// cobra validates Args before any hook: an arity failure used to return
// before anything was logged (ako/mxcli#633).
//

This comment correctly explains why the change was made.

In diag_loop_report_test.go, there are comprehensive tests for the PID-based pairing.

In the listview editable inputs test, I see:

{
			name: "quoted 'true' is written false",
			src: `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) {
  listview lv (datasource: database M.Thing, editable: 'true') {
    textbox t (label: 'N', attribute: Name)
  }
+}`,
+		

---
*Automated review via OpenRouter (Nemotron Super 120B) — [workflow source](https://github.com/mendixlabs/mxcli/blob/main/.github/workflows/ai-review.yml)*

@ako
ako merged commit 57259e8 into mendixlabs:main Sep 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment