diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json new file mode 100644 index 000000000..904f4b6d3 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-09","symptom":"`create or replace navigation` turning page item 'Work' into a sub-menu 'Work' reports success (plus `kept the stored action of menu item 'Work' (Forms$FormAction): MDL cannot express it`), then `mx check` fails with CE0548 \"Items with subitems cannot have an action themselves.\" (v0.25.0 regression; a different sub-menu caption avoids it)","cause":"`keepStoredMenuAction` had a sub-menu branch that kept ANY stored non-NoAction, on the reasoning that a sub-menu \"takes no OnClick, so a stored action on one is never stated\". Pairing is by caption only, so the stored page item's action was carried onto the new sub-menu. Navigation profiles and menu documents share the decision","file":"`mdl/executor/cmd_navigation.go` (`keepStoredMenuAction`)","insight":"**A keep-what-MDL-cannot-say rule must only keep what the target element can legally hold.** \"The script states no action\" is not \"the script wants the stored one\" when the element's kind changed under the same caption — a sub-menu can hold no action at all, so the carry is never right there. The tell in the output was the false reason: `Forms$FormAction` is printable MDL, yet the message said MDL cannot express it. Guards `TestNavigationRewrite_SubMenuDoesNotKeepAStoredAction` / `TestCreateMenu_SubMenuDoesNotKeepAStoredAction` (control: a leaf still keeps an unprintable action, #980). Measured on 11.14.0 with `mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl`: unfixed binary -> docker check CE0548; fixed -> 0 errors","refs":["mendixlabs/mxcli#1341","ako/mxcli#980"],"ce":["CE0548"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd90ddcd..68988b1d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A navigation sub-menu no longer keeps the action of the page item it replaces** (mendixlabs/mxcli#1341) — turning menu item `'Work'` into sub-menu `'Work'` with `create or replace navigation` (or `create or modify menu`) paired the two by caption and carried the page item's `Forms$FormAction` onto the sub-menu, reporting it as an action "MDL cannot express"; `mx check` then failed with CE0548 "Items with subitems cannot have an action themselves." A sub-menu is now always written with no action (measured on 11.14.0). - **`mxcli test --attach` and the dev loop's "already serving" detection see a live app on Windows** (mendixlabs/mxcli#1284) — both read a pid from a handshake file (`.mxcli/test-endpoint.json`, `.mxcli/run-local.json`) and tested it with `Signal(0)`, which Go supports only as Kill on Windows, so every pid read as dead: `test --attach` always refused with "the app that published … is no longer running" while the app was up, and a command that looks for a dev loop already serving the project (the recompile warning, for one) never found it. Liveness is now `internal/procalive.Alive`, which uses OpenProcess and WaitForSingleObject on Windows (the check `run --local` already used for mxbuild) and `Signal(0)` elsewhere. No change on Linux or macOS. - **`run stop` on Windows no longer reports a run that has shut down as still alive** — it printed "failed: 1 process(es) of pid N's run are still alive" and exited 1 for a run that had stopped, because its liveness check counted any pid Windows could still open as alive, and an exited process stays openable while its parent holds a handle to it. It now uses the same check as `test --attach` (above), and reports "stopped: pid N". - **`retrieve $u from $obj/System.owner` passed check and exec, then failed the build with CE0136** (mendixlabs/mxcli#1358) — `owner` and `changedBy` are entity flags, not modelled associations: mxbuild resolves the name in a retrieve by association but derives no entity from it, `[CE0136] "Retrieve object must specify the 'Entity' property."`, with or without `owner: AutoOwner` (measured on 11.12.2, `System.changedBy` alike). `check` and `exec` now refuse it as **MDL-RETRIEVE02** and name the form that builds: `retrieve $u from System.User where [id = $obj/System.owner] first;`. diff --git a/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl b/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl new file mode 100644 index 000000000..bb213a2c4 --- /dev/null +++ b/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl @@ -0,0 +1,42 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1341: a sub-menu does not keep a same-caption item's action +-- ============================================================================ +-- +-- Turning the page item 'Work' into a sub-menu 'Work' paired the two by caption +-- and carried the page item's Forms$FormAction onto the sub-menu (v0.25.0): +-- +-- kept the stored action of menu item 'Work' (Forms$FormAction): MDL cannot +-- express it, and the item states none +-- +-- exec and describe reported success; mx check then failed with CE0548 "Items +-- with subitems cannot have an action themselves." The sub-menu must be written +-- with Forms$NoAction, and `mxcli docker check` must report 0 errors after the +-- second statement. + +create module NavSubmenu1341; + +create page NavSubmenu1341.Accounts ( Title: 'Accounts', Layout: Atlas_Core.Atlas_Default ) { + dynamictext t (Content: 'Accounts') +}; + +create page NavSubmenu1341.Sessions ( Title: 'Sessions', Layout: Atlas_Core.Atlas_Default ) { + dynamictext t (Content: 'Sessions') +}; + +-- Step 1: 'Work' is a page item. +create or modify navigation Responsive + home page NavSubmenu1341.Accounts + { + menu item 'Work' ( OnClick: show page NavSubmenu1341.Accounts ) + }; + +-- Step 2: 'Work' becomes a sub-menu under the same caption. +create or modify navigation Responsive + home page NavSubmenu1341.Accounts + { + menu 'Work' { + menu item 'Accounts' ( OnClick: show page NavSubmenu1341.Accounts ) + menu item 'Sessions' ( OnClick: show page NavSubmenu1341.Sessions ) + } + }; diff --git a/mdl/executor/cmd_navigation.go b/mdl/executor/cmd_navigation.go index e857e2ff1..a01f84447 100644 --- a/mdl/executor/cmd_navigation.go +++ b/mdl/executor/cmd_navigation.go @@ -261,11 +261,11 @@ func keepStoredMenuAction(ctx *ExecContext, def ast.NavMenuItemDef, match *types if menuItemStatesAction(def) || len(match.StoredAction) == 0 { return false, "" } - // A sub-menu takes no OnClick, so a stored action on one is never stated. + // A sub-menu cannot have an action at all — mx check CE0548 "Items with + // subitems cannot have an action themselves" — so nothing stored is kept on + // it, least of all a page item's action paired by caption + // (mendixlabs/mxcli#1341). if len(def.Items) > 0 { - if t := menuActionTypeName(match); !isNoMenuAction(t) && t != "NoAction" { - return true, t - } return false, "" } if _, note := menuItemActionMDL(ctx, match); note != "" { diff --git a/mdl/executor/issue1341_submenu_keeps_page_action_test.go b/mdl/executor/issue1341_submenu_keeps_page_action_test.go new file mode 100644 index 000000000..1e93172e3 --- /dev/null +++ b/mdl/executor/issue1341_submenu_keeps_page_action_test.go @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1341: "CREATE OR REPLACE NAVIGATION keeps a page item's +// action on a same-caption sub-menu (mx check CE0548; v0.25.0 regression)" — +// CE0548 "Items with subitems cannot have an action themselves". The rewrite +// paired the new sub-menu 'Work' with the stored page item 'Work' by caption +// and carried its Forms$FormAction, because a sub-menu "states no action". +// A sub-menu cannot have one, so nothing stored may be kept on it. + +// storedWorkPageItem is the menu as v0.25.0 read it back: a top-level page +// item 'Work' (a Forms$FormAction, which describe prints) beside 'Reports', +// whose action describe cannot print. +func storedWorkPageItem(pageAction, unknown []byte) []*types.NavMenuItem { + return []*types.NavMenuItem{ + {Caption: "Work", ActionType: "PageAction", Page: "Administration.Account_Overview", + StoredAction: pageAction, ActionDoc: map[string]any{"$Type": "Forms$FormAction"}}, + {Caption: "Reports", ActionType: "Forms$UnknownFutureClientAction", + StoredAction: unknown, ActionDoc: map[string]any{"$Type": "Forms$UnknownFutureClientAction"}}, + } +} + +func TestNavigationRewrite_SubMenuDoesNotKeepAStoredAction(t *testing.T) { + pageAction := []byte("stored-form-action") + unknown := []byte("stored-unknown-action") + var got types.NavigationProfileSpec + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{Profiles: []*types.NavigationProfile{{ + Name: "Responsive", Kind: "Responsive", MenuItems: storedWorkPageItem(pageAction, unknown), + }}}, nil + }, + UpdateNavigationProfileFunc: func(_ model.ID, _ string, spec types.NavigationProfileSpec) error { + got = spec + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + prog, errs := visitor.Build(`create or replace navigation Responsive + menu ( + menu 'Work' ( + menu item 'Accounts' page Administration.Account_Overview; + menu item 'Sessions' page Administration.ActiveSessions; + ); + menu item 'Reports'; + ) +;`) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + assertNoError(t, execAlterNavigation(ctx, prog.Statements[0].(*ast.AlterNavigationStmt))) + + if len(got.MenuItems) != 2 || got.MenuItems[0].Caption != "Work" { + t.Fatalf("spec items = %+v, want 'Work' and 'Reports'", got.MenuItems) + } + work := got.MenuItems[0] + if work.KeepAction != nil { + t.Errorf("sub-menu 'Work' kept the stored page action %q: mx check CE0548 "+ + "\"Items with subitems cannot have an action themselves\"", work.KeepAction) + } + if len(work.Items) != 2 { + t.Errorf("sub-menu 'Work' has %d items, want 2", len(work.Items)) + } + if strings.Contains(buf.String(), "menu item 'Work'") { + t.Errorf("no action is kept on 'Work', so none may be reported:\n%s", buf.String()) + } + // CONTROL: a leaf that states no action still keeps one MDL cannot print + // (#980). A converter that kept nothing at all would pass the checks above. + if !bytes.Equal(got.MenuItems[1].KeepAction, unknown) { + t.Errorf("leaf 'Reports' must keep its stored action, got KeepAction=%q", got.MenuItems[1].KeepAction) + } +} + +// The menu-document writer shares the decision (menuItemsFromAST). +func TestCreateMenu_SubMenuDoesNotKeepAStoredAction(t *testing.T) { + pageAction := []byte("stored-form-action") + unknown := []byte("stored-unknown-action") + existing := &types.MenuDocument{ID: "menu-1", ContainerID: "folder-9", Name: "Main_Menu", + Items: storedWorkPageItem(pageAction, unknown)} + var updated *types.MenuDocument + mb := menuBackend(existing) + mb.GetModuleByNameFunc = func(name string) (*model.Module, error) { + return &model.Module{BaseElement: model.BaseElement{ID: "mod-1"}, Name: name}, nil + } + mb.UpdateMenuDocumentFunc = func(md *types.MenuDocument) error { updated = md; return nil } + + ctx, _ := newMockCtx(t, withBackend(mb)) + assertNoError(t, execCreateMenu(ctx, &ast.CreateMenuStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "Main_Menu"}, + CreateOrModify: true, + Items: []ast.NavMenuItemDef{ + {Caption: "Work", Items: []ast.NavMenuItemDef{{Caption: "Accounts"}}}, + {Caption: "Reports"}, + }, + })) + + if updated == nil || len(updated.Items) != 2 { + t.Fatalf("UpdateMenuDocument got %+v", updated) + } + if work := updated.Items[0]; work.StoredAction != nil || work.ActionType != "NoAction" { + t.Errorf("sub-menu 'Work' kept ActionType=%q StoredAction=%q: CE0548", work.ActionType, work.StoredAction) + } + // CONTROL: the leaf keeps what MDL cannot print. + if !bytes.Equal(updated.Items[1].StoredAction, unknown) { + t.Errorf("leaf 'Reports' must keep its stored action, got %q", updated.Items[1].StoredAction) + } +}