From 79faf2d87833ded335c8ead8e05571780331a6e1 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 17:42:56 +0000 Subject: [PATCH 1/3] fix: count workflow activities with one walk in list workflows, show structure and the catalog (#963) list workflows and show structure recursed over outcome flows only and skipped boundary-event flows and event sub-processes, so TestApp Workflow1 listed 5 activities where the catalog counted 8. The catalog's walk moves to wfnames.WalkActivities/CountActivities and all three use it. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + mdl/backend/wfnames/walk.go | 69 +++++++++++++ mdl/catalog/builder_workflows.go | 17 +--- mdl/catalog/workflow_walk.go | 39 +------- mdl/executor/cmd_structure.go | 53 +--------- mdl/executor/cmd_workflows.go | 63 +----------- mdl/executor/cmd_workflows_count_test.go | 99 +++++++++++++++++++ 8 files changed, 185 insertions(+), 157 deletions(-) create mode 100644 mdl/backend/wfnames/walk.go create mode 100644 mdl/executor/cmd_workflows_count_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 4dc21c1e0a..045a13388d 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -850,3 +850,4 @@ {"area": "mdl/executor", "date": "2026-10-03", "symptom": "ako/mxcli#950 item 1: a page button described as `Action: delete close page` executes to a Forms$DeleteClientAction with ClosePage=false — a describe → exec round trip silently stops the button closing its page; check, exec and mx check are all clean", "cause": "buildClientActionV3Base's `delete` case did not copy action.ClosePage, although the visitor sets it and the writer (clientActionToGen) writes it; save/cancel copied it", "file": "`mdl/executor/cmd_pages_builder_v3.go` (buildClientActionV3Base, case \"delete\")", "insight": "The audit of the other cases found no other dropped visitor flag, but two hard-coded ones describe cannot express: complete task always writes ClosePage/Commit true and show page/create object write NumberOfPagesToClose2 \"\" — every Studio Pro instance in TestApp/PedApp has those values, so they are latent, not live. A text round trip could not have caught this: describe printed the flag correctly and the second describe of the exec'd page printed `delete`, which looks like a user edit — read the stored ClosePage", "refs": ["ako/mxcli#950"]} {"area": "mdl/executor", "date": "2026-10-03", "symptom": "ako/mxcli#950 item 2: describe of a flow ending `return [%CurrentUser%];` prints `return $[%CurrentUser%];`, which does not parse; `return if … then … else …` likewise became `return $if …` (two TestApp WorkflowCommons microflows)", "cause": "formatActivity's EndEvent branch added `$` to any return value without one of + ' \" ( ) — a character blacklist standing in for 'is a bare variable name'", "file": "`mdl/executor/cmd_microflows_format_action.go` (isBareReturnVariable)", "insight": "Restore a stripped sigil only for the positive shape it was stripped from (a bare name, optionally /path); a blacklist of characters lets every new expression form through. The other `$`-adding sites in describe prefix variable-NAME fields, not expressions, and are safe", "refs": ["ako/mxcli#950"]} {"area": "mdl/executor", "date": "2026-10-03", "symptom": "ako/mxcli#950 item 3: describe of a navigation list prints item actions as `show_page 'Mod.Page'` (does not parse — TestApp Rules.Entity_Menu, 3 syntax errors); with that fixed, exec refuses the description with `item inside navigationlist requires a name` because Studio Pro leaves items unnamed", "cause": "extractNavigationListItemAction had a private copy of the page-action rendering in the legacy form instead of the shared renderClientActionMDL; buildNavigationListItemV3 required a name Studio Pro never stores; and once exec accepted it, the writer wrote `Name: \"\"` and no ConditionalVisibilitySettings where Studio Pro stores no Name key and a null slot (6 of 6 items in TestApp), so GetPut still rewrote the snippet", "file": "`mdl/executor/cmd_pages_describe_parse.go` (extractNavigationListItemAction), `mdl/executor/cmd_pages_builder_v3_widgets.go` (buildNavigationListItemV3), `mdl/executor/cmd_pages_describe_output.go` (item header), `mdl/backend/modelsdk/widget_write.go` (navListItemToGen, Forms$NavigationListItem NullFields)", "insight": "Fixing the reported parse error only exposed the next law: the issue said the empty item name 'parses fine', which was true and irrelevant — exec refused it, and after that the writer rewrote it. An unnamed item with no Name key passes mx check at 11.14.0, contrary to the old ledger note that the key is mandatory (that applies to a NAMED item's key, not its absence). Run the whole describe → check → exec → describe chain on the Studio Pro-authored document before declaring a round-trip bug fixed", "refs": ["ako/mxcli#950"]} +{"area": "mdl/executor", "date": "2026-10-03", "symptom": "`list workflows` and `show structure` report fewer workflow activities than the catalog's workflows_data (TestApp Workflow1: 5 vs 8)", "cause": "cmd_workflows.go countFlowActivities and cmd_structure.go countStructureFlowActivities each recursed over outcome flows only, skipping boundary-event flows and event sub-processes; the catalog had moved to a shared walk in #937 and the executor copies were left behind", "file": "`mdl/backend/wfnames/walk.go` (`WalkActivities`, `CountActivities`), `mdl/executor/cmd_workflows.go`, `mdl/executor/cmd_structure.go`, `mdl/catalog/workflow_walk.go`", "insight": "Duplicate-resolver drift: fixing one copy of a traversal (#937) left two private copies answering differently. The walk now lives in wfnames, which both catalog and executor already import, so there is one place to add a new sub-flow slot. Grep for every recursion over `workflows.Flow` when one is fixed.", "refs": ["ako/mxcli#963", "ako/mxcli#937"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac2b20156..c9298c0f4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`list workflows` and `show structure` count every activity of a workflow** (ako/mxcli#963) — including those on a boundary-event path and in an event sub-process, as the catalog's `workflows_data` has since #937. TestApp `Workflow1` listed 5 activities where the catalog counted 8; all three now share one walk (`wfnames.WalkActivities`). - **`docker check` no longer modifies the project** (ako/mxcli#951) — `mx update-widgets` and `mx check` now run on a temporary copy, for MPR v1 and v2 alike, and what mx prints names the project's own paths. Before, a check rewrote an MPR v1 project's `.mpr` permanently (only v2 was restored from a snapshot), and `mx check` itself rewrote `theme-cache/` and created `deployment/sass/` even with `--no-update-widgets`. The output now says that widget definitions were normalised on a copy, and that a CE0463 the stored project still has is therefore not reported: `--no-update-widgets` checks the project as stored, `mxcli fix widgets` applies the normalisation (ako/mxcli#568, #646). Build output, caches and VCS folders are not copied; the copy goes to `$TMPDIR` and is removed afterwards. - **Parallel mxcli runs on one project no longer fail to save the catalog cache** (ako/mxcli#951) — eight parallel `lint` runs on a fresh copy printed `failed to create table catalog_meta: table catalog_meta already exists` or `database is locked`. The cache is now written to a temporary file next to it and renamed into place, so every run saves and a reader sees the old cache or the new one, never a half-written file; opening a current cache no longer writes to it. - **A lint rule that fails no longer costs the project score** (ako/mxcli#952) — a Starlark rule reading a struct field this mxcli does not expose (a rule written for a newer mxcli, such as one using `document_noun_title` under v0.24.0) is reported at info level as `rule needs a newer mxcli ()` instead of an error. All rule failures are kept out of `mxcli report`'s score, summary and categories and listed in their own "Rules That Could Not Run" section (`ruleFailures` in JSON); other failures stay `Starlark rule error` errors in `mxcli lint`. A configured rule severity no longer applies to the rule's own failure. A rule file that fails to load on an undefined name says the rule may need a newer mxcli. Measured on PedApp with v0.24.0 and rules from main: QUAL004 and CUSTOM002 crashed and scored as 2 errors. diff --git a/mdl/backend/wfnames/walk.go b/mdl/backend/wfnames/walk.go new file mode 100644 index 0000000000..f4616ae2f1 --- /dev/null +++ b/mdl/backend/wfnames/walk.go @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 + +package wfnames + +import "github.com/mendixlabs/mxcli/sdk/workflows" + +// WalkActivities visits every activity of a workflow, depth-first: the main +// flow, every sub-flow an activity carries (outcome flows and boundary-event +// flows, per SubFlows), and every event sub-process. +// +// It is the one traversal every reader of a stored workflow shares — the +// catalog's refs and activity counts, `list workflows` and `show structure`. +// Each used to carry its own recursion over outcomes only, so they disagreed: +// a microflow called from a boundary-event path had no inbound edge +// (mendixlabs/mxcli#1269), and `list workflows` counted 5 activities where the +// catalog counted 8 (ako/mxcli#963). A new place that holds a flow is added to +// SubFlows or here, and every caller sees it. +func WalkActivities(wf *workflows.Workflow, visit func(workflows.WorkflowActivity)) { + if wf == nil { + return + } + walkFlow(wf.Flow, visit) + for _, esp := range wf.EventSubProcesses { + if esp != nil { + walkFlow(esp.Flow, visit) + } + } +} + +func walkFlow(flow *workflows.Flow, visit func(workflows.WorkflowActivity)) { + if flow == nil { + return + } + for _, act := range flow.Activities { + if act == nil { + continue + } + visit(act) + for _, sub := range SubFlows(act) { + walkFlow(sub, visit) + } + } +} + +// ActivityCounts is the per-type tally of a workflow's activities over every +// flow WalkActivities reaches. +type ActivityCounts struct { + Total int + UserTasks int + MicroflowCalls int // call-microflow and system tasks + Decisions int +} + +// CountActivities tallies a workflow's activities with WalkActivities. +func CountActivities(wf *workflows.Workflow) ActivityCounts { + var c ActivityCounts + WalkActivities(wf, func(act workflows.WorkflowActivity) { + c.Total++ + switch act.(type) { + case *workflows.UserTask: + c.UserTasks++ + case *workflows.CallMicroflowTask, *workflows.SystemTask: + c.MicroflowCalls++ + case *workflows.ExclusiveSplitActivity: + c.Decisions++ + } + }) + return c +} diff --git a/mdl/catalog/builder_workflows.go b/mdl/catalog/builder_workflows.go index 0405a6796d..0ff45c8a74 100644 --- a/mdl/catalog/builder_workflows.go +++ b/mdl/catalog/builder_workflows.go @@ -3,6 +3,7 @@ package catalog import ( + "github.com/mendixlabs/mxcli/mdl/backend/wfnames" "github.com/mendixlabs/mxcli/sdk/workflows" ) @@ -71,18 +72,8 @@ func (b *Builder) buildWorkflows() error { } // countWorkflowActivityTypes counts activity types in a workflow, over every -// flow it holds (see walkWorkflowActivities). +// flow it holds (see wfnames.WalkActivities). func countWorkflowActivityTypes(wf *workflows.Workflow) (total, userTasks, microflowCalls, decisions int) { - walkWorkflowActivities(wf, func(act workflows.WorkflowActivity) { - total++ - switch act.(type) { - case *workflows.UserTask: - userTasks++ - case *workflows.CallMicroflowTask, *workflows.SystemTask: - microflowCalls++ - case *workflows.ExclusiveSplitActivity: - decisions++ - } - }) - return + c := wfnames.CountActivities(wf) + return c.Total, c.UserTasks, c.MicroflowCalls, c.Decisions } diff --git a/mdl/catalog/workflow_walk.go b/mdl/catalog/workflow_walk.go index a0fb0f28c7..00ba51824c 100644 --- a/mdl/catalog/workflow_walk.go +++ b/mdl/catalog/workflow_walk.go @@ -7,43 +7,6 @@ import ( "github.com/mendixlabs/mxcli/sdk/workflows" ) -// walkWorkflowActivities visits every activity of a workflow, depth-first: the -// main flow, every sub-flow an activity carries (outcome flows and -// boundary-event flows, per wfnames.SubFlows), and every event sub-process. -// -// It is the one traversal the catalog's workflow passes share. The refs walk and -// the activity counts each had their own recursion over outcomes only, so a -// microflow called from a boundary-event path or an event sub-process had no -// inbound edge and was reported dead by graph_dead_assets (mendixlabs/mxcli#1269, -// workflows row). A new place that holds a flow is added to wfnames.SubFlows or -// here, and both passes see it. -func walkWorkflowActivities(wf *workflows.Workflow, visit func(workflows.WorkflowActivity)) { - if wf == nil { - return - } - walkWorkflowFlow(wf.Flow, visit) - for _, esp := range wf.EventSubProcesses { - if esp != nil { - walkWorkflowFlow(esp.Flow, visit) - } - } -} - -func walkWorkflowFlow(flow *workflows.Flow, visit func(workflows.WorkflowActivity)) { - if flow == nil { - return - } - for _, act := range flow.Activities { - if act == nil { - continue - } - visit(act) - for _, sub := range wfnames.SubFlows(act) { - walkWorkflowFlow(sub, visit) - } - } -} - // workflowDocRef is one reference a workflow makes to another document. type workflowDocRef struct { TargetType, TargetName, RefKind string @@ -75,7 +38,7 @@ func workflowDocRefs(wf *workflows.Workflow) []workflowDocRef { } } - walkWorkflowActivities(wf, func(act workflows.WorkflowActivity) { + wfnames.WalkActivities(wf, func(act workflows.WorkflowActivity) { switch a := act.(type) { case *workflows.UserTask: add(RefObjectPage, a.Page, RefKindShowPage) diff --git a/mdl/executor/cmd_structure.go b/mdl/executor/cmd_structure.go index 4501445d89..dd22d89132 100644 --- a/mdl/executor/cmd_structure.go +++ b/mdl/executor/cmd_structure.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/wfnames" "github.com/mendixlabs/mxcli/mdl/catalog" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/types" @@ -1016,55 +1017,11 @@ func structureWorkflows(ctx *ExecContext, moduleName string, wfs []*workflows.Wo } } -// countStructureWorkflowActivities counts activity types in a workflow for structure output. +// countStructureWorkflowActivities counts activity types in a workflow for +// structure output, over every flow it holds — the catalog's walk (wfnames). func countStructureWorkflowActivities(wf *workflows.Workflow) (total, userTasks, microflowCalls, decisions int) { - if wf.Flow == nil { - return - } - countStructureFlowActivities(wf.Flow, &total, &userTasks, µflowCalls, &decisions) - return -} - -// countStructureFlowActivities recursively counts activity types in a flow. -func countStructureFlowActivities(flow *workflows.Flow, total, userTasks, microflowCalls, decisions *int) { - if flow == nil { - return - } - for _, act := range flow.Activities { - *total++ - switch a := act.(type) { - case *workflows.UserTask: - *userTasks++ - for _, outcome := range a.Outcomes { - countStructureFlowActivities(outcome.Flow, total, userTasks, microflowCalls, decisions) - } - case *workflows.CallMicroflowTask: - *microflowCalls++ - for _, outcome := range a.Outcomes { - if outcome != nil { - countStructureFlowActivities(outcome.GetFlow(), total, userTasks, microflowCalls, decisions) - } - } - case *workflows.SystemTask: - *microflowCalls++ - for _, outcome := range a.Outcomes { - if outcome != nil { - countStructureFlowActivities(outcome.GetFlow(), total, userTasks, microflowCalls, decisions) - } - } - case *workflows.ExclusiveSplitActivity: - *decisions++ - for _, outcome := range a.Outcomes { - if outcome != nil { - countStructureFlowActivities(outcome.GetFlow(), total, userTasks, microflowCalls, decisions) - } - } - case *workflows.ParallelSplitActivity: - for _, outcome := range a.Outcomes { - countStructureFlowActivities(outcome.Flow, total, userTasks, microflowCalls, decisions) - } - } - } + c := wfnames.CountActivities(wf) + return c.Total, c.UserTasks, c.MicroflowCalls, c.Decisions } // ============================================================================ diff --git a/mdl/executor/cmd_workflows.go b/mdl/executor/cmd_workflows.go index 318fed2a0e..6fe941c001 100644 --- a/mdl/executor/cmd_workflows.go +++ b/mdl/executor/cmd_workflows.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/wfnames" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/visitor" "github.com/mendixlabs/mxcli/sdk/workflows" @@ -70,65 +71,11 @@ func listWorkflows(ctx *ExecContext, moduleName string) error { return writeResult(ctx, result) } -// countWorkflowActivities counts total activities, user tasks, and decisions in a workflow. +// countWorkflowActivities counts total activities, user tasks, and decisions in +// a workflow, over every flow it holds — the catalog's walk (wfnames). func countWorkflowActivities(wf *workflows.Workflow) (total, userTasks, decisions int) { - if wf.Flow == nil { - return - } - countFlowActivities(wf.Flow, &total, &userTasks, &decisions) - return -} - -// countFlowActivities recursively counts activities in a flow and its sub-flows. -func countFlowActivities(flow *workflows.Flow, total, userTasks, decisions *int) { - if flow == nil { - return - } - for _, act := range flow.Activities { - *total++ - switch a := act.(type) { - case *workflows.UserTask: - *userTasks++ - for _, outcome := range a.Outcomes { - countFlowActivities(outcome.Flow, total, userTasks, decisions) - } - case *workflows.ExclusiveSplitActivity: - *decisions++ - for _, outcome := range a.Outcomes { - if co, ok := outcome.(*workflows.BooleanConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.EnumerationValueConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.VoidConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } - } - case *workflows.ParallelSplitActivity: - for _, outcome := range a.Outcomes { - countFlowActivities(outcome.Flow, total, userTasks, decisions) - } - case *workflows.CallMicroflowTask: - for _, outcome := range a.Outcomes { - if co, ok := outcome.(*workflows.BooleanConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.EnumerationValueConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.VoidConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } - } - case *workflows.SystemTask: - for _, outcome := range a.Outcomes { - if co, ok := outcome.(*workflows.BooleanConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.EnumerationValueConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } else if co, ok := outcome.(*workflows.VoidConditionOutcome); ok { - countFlowActivities(co.Flow, total, userTasks, decisions) - } - } - } - } + c := wfnames.CountActivities(wf) + return c.Total, c.UserTasks, c.Decisions } // describeWorkflow handles DESCRIBE WORKFLOW command. diff --git a/mdl/executor/cmd_workflows_count_test.go b/mdl/executor/cmd_workflows_count_test.go new file mode 100644 index 0000000000..beeb7de0cf --- /dev/null +++ b/mdl/executor/cmd_workflows_count_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/workflows" +) + +// `list workflows` and `show structure` count a workflow's activities with the +// same walk as the catalog (ako/mxcli#963): activities on a boundary-event path +// and in an event sub-process are activities of the workflow. +func boundaryAndESPWorkflow() *workflows.Workflow { + call := func(mf string) *workflows.CallMicroflowTask { return &workflows.CallMicroflowTask{Microflow: mf} } + return &workflows.Workflow{ + Name: "WF", + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{ + &workflows.UserTask{ + BoundaryEvents: []*workflows.BoundaryEvent{{ + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{call("M.InBoundary")}}, + }}, + }, + &workflows.ExclusiveSplitActivity{}, + }}, + EventSubProcesses: []*workflows.EventSubProcess{{ + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{ + &workflows.EventSubProcessStartActivity{}, + &workflows.UserTask{}, + }}, + }}, + } +} + +// outcomeOnlyWorkflow is the control: no boundary events, no event +// sub-process, so the old outcome-only recursion and the shared walk agree. +func outcomeOnlyWorkflow() *workflows.Workflow { + return &workflows.Workflow{ + Name: "WF", + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{ + &workflows.UserTask{Outcomes: []*workflows.UserTaskOutcome{{ + Flow: &workflows.Flow{Activities: []workflows.WorkflowActivity{&workflows.CallMicroflowTask{}}}, + }}}, + &workflows.ExclusiveSplitActivity{}, + }}, + } +} + +func TestCountWorkflowActivities_BoundaryAndEventSubProcess(t *testing.T) { + cases := []struct { + name string + wf *workflows.Workflow + total, ut, calls, decs int + }{ + // main: user task, decision = 2; boundary: call = 1; ESP: start, user task = 2 + {"boundary+esp", boundaryAndESPWorkflow(), 5, 2, 1, 1}, + {"control: outcomes only", outcomeOnlyWorkflow(), 3, 1, 1, 1}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + total, ut, decs := countWorkflowActivities(c.wf) + if total != c.total || ut != c.ut || decs != c.decs { + t.Errorf("list workflows counts = (%d, %d, %d), want (%d, %d, %d)", total, ut, decs, c.total, c.ut, c.decs) + } + stotal, sut, scalls, sdecs := countStructureWorkflowActivities(c.wf) + if stotal != c.total || sut != c.ut || scalls != c.calls || sdecs != c.decs { + t.Errorf("show structure counts = (%d, %d, %d, %d), want (%d, %d, %d, %d)", stotal, sut, scalls, sdecs, c.total, c.ut, c.calls, c.decs) + } + }) + } +} + +// On TestApp, Workflow1 has boundary events: the catalog has always counted 8 +// activities for it, `list workflows` counted 5. Every other workflow is the +// control — it must count the same either way. +func TestCountWorkflowActivities_TestAppWorkflow1(t *testing.T) { + exec, _ := openTestAppCopy(t) + wfs, err := exec.backend.ListWorkflows() + if err != nil { + t.Fatal(err) + } + found := false + for _, wf := range wfs { + total, _, _ := countWorkflowActivities(wf) + stotal, _, _, _ := countStructureWorkflowActivities(wf) + if stotal != total { + t.Errorf("%s: show structure %d != list workflows %d", wf.Name, stotal, total) + } + if wf.Name == "Workflow1" { + found = true + if total != 8 { + t.Errorf("Workflow1 activities = %d, want 8 (the catalog's count)", total) + } + } + } + if !found { + t.Fatal("TestApp has no Workflow1") + } +} From 493bb7ce8bb00d51eddf2d8668809fbc31ca7165 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 17:46:03 +0000 Subject: [PATCH 2/3] feat(lint): expose total_activity_count on the Starlark microflow struct (#963) The catalog has carried TotalActivityCount (loop bodies included) since #940; microflows() now hands it to rules for microflows, nanoflows and rules alike. Co-Authored-By: Claude Opus 5.5 --- .../skills/mendix/write-lint-rules/SKILL.md | 3 +- CHANGELOG.md | 1 + mdl/linter/context.go | 11 ++-- mdl/linter/context_document_filter_test.go | 2 +- mdl/linter/context_test.go | 4 +- mdl/linter/rules/empty_test.go | 6 +- mdl/linter/starlark.go | 4 +- mdl/linter/starlark_documented_values_test.go | 8 +-- mdl/linter/starlark_flow_noun_test.go | 14 ++-- .../starlark_total_activity_count_test.go | 66 +++++++++++++++++++ 10 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 mdl/linter/starlark_total_activity_count_test.go diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 3b34fc88ac..9914e5cb6e 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -205,7 +205,8 @@ def check(): | `description` | string | Documentation text | | `return_type` | string | Return type | | `parameter_count` | int | Number of parameters | -| `activity_count` | int | Number of activities | +| `activity_count` | int | Number of activities at the top level of the flow, excluding start/end events and merges. A loop counts as one; its body is not counted | +| `total_activity_count` | int | `activity_count` plus every activity inside a loop, at any depth — the size of the flow including loop bodies. Equal to `activity_count` for a flow without loops | | `complexity` | int | McCabe cyclomatic complexity | | `document_noun` | string | `"microflow"`, `"nanoflow"` or `"rule"` — for mid-sentence use in a message | | `document_noun_title` | string | `"Microflow"`, `"Nanoflow"` or `"Rule"` — for `document_type=` and a message that opens with it | diff --git a/CHANGELOG.md b/CHANGELOG.md index c9298c0f4b..a2d2244ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **`total_activity_count` on the Starlark microflow struct** (ako/mxcli#963) — the catalog's `TotalActivityCount` (loop bodies included, at any depth) for every flow `microflows()` yields: microflows, nanoflows and rules. `activity_count` keeps counting a loop as one activity. - **A project records which mxcli wrote its tooling, and an older binary says so** (ako/mxcli#952) — `mxcli init` and every `init --sync-skills` write `.ai-context/mxcli-tooling.json` (version, build time, date; rewritten only when the version changes). Any command that opens the project with `-p` and a binary **older** than the stamp warns once on stderr, naming both versions and how to update; `init --sync-skills` from an older binary **refuses** instead of rolling the skills, rules and CLAUDE.md back. Releases compare by number, nightlies by tag date, a release against a nightly by build date; dev builds are never reported. Binaries from v0.24.0 and earlier cannot read the stamp, so the regenerated `.claude/bootstrap-mxcli.sh` checks it before choosing a binary: an older `mxcli` on PATH is not linked in (it downloads `MXCLI_TAG` instead), an older `./mxcli` is replaced, and the download lands through a temporary file so a `./mxcli` symlink never has it written through into the PATH binary. - **`init --sync-skills` (alias `--sync`) refreshes the bundled lint rules and the mxcli section of CLAUDE.md / AGENTS.md** (ako/mxcli#952) — it used to refresh only the skills, so a project kept the lint rules and guidance of whichever mxcli first initialised it. Bundled rules are recognised by file name; your own rules beside them are never touched. CLAUDE.md and AGENTS.md are now written between `` / `` markers, and only that section is refreshed — by the sync and by a re-run of `mxcli init` — so project notes outside the markers survive. A file written before the markers is left alone by the sync, with a note; run `mxcli init` once to adopt them. - **`mxcli init` and `mxcli new` create `mdlsource/`** (ako/mxcli#952) — the directory the generated CLAUDE.md says scripts live in, with a README. diff --git a/mdl/linter/context.go b/mdl/linter/context.go index f72d76276f..459a417763 100644 --- a/mdl/linter/context.go +++ b/mdl/linter/context.go @@ -599,8 +599,11 @@ type Microflow struct { Description string ReturnType string ParameterCount int - ActivityCount int - Complexity int // McCabe cyclomatic complexity + ActivityCount int // top level only; a loop counts as one + // TotalActivityCount is ActivityCount plus every activity inside a loop, + // at any depth (mendixlabs/mxcli#1266). + TotalActivityCount int + Complexity int // McCabe cyclomatic complexity } // DocumentNoun is what to call this document in a lint message and in @@ -636,7 +639,7 @@ func (ctx *LintContext) Microflows() iter.Seq[Microflow] { rows, err := ctx.db.Query(fmt.Sprintf(` SELECT mf.Id, mf.Name, mf.QualifiedName, mf.ModuleName, mf.Folder, mf.MicroflowType, mf.Description, mf.ReturnType, - mf.ParameterCount, mf.ActivityCount, mf.Complexity + mf.ParameterCount, mf.ActivityCount, mf.TotalActivityCount, mf.Complexity FROM microflows mf LEFT JOIN modules m ON mf.ModuleName = m.Name WHERE %s AND %s @@ -652,7 +655,7 @@ func (ctx *LintContext) Microflows() iter.Seq[Microflow] { var mf Microflow var desc, retType, folder sql.NullString err := rows.Scan(&mf.ID, &mf.Name, &mf.QualifiedName, &mf.ModuleName, &folder, - &mf.MicroflowType, &desc, &retType, &mf.ParameterCount, &mf.ActivityCount, &mf.Complexity) + &mf.MicroflowType, &desc, &retType, &mf.ParameterCount, &mf.ActivityCount, &mf.TotalActivityCount, &mf.Complexity) if err != nil { ctx.recordQueryError("Microflows (row scan)", err) continue diff --git a/mdl/linter/context_document_filter_test.go b/mdl/linter/context_document_filter_test.go index caa921795a..48543b2247 100644 --- a/mdl/linter/context_document_filter_test.go +++ b/mdl/linter/context_document_filter_test.go @@ -31,7 +31,7 @@ import ( func twoInOneModuleDB(t *testing.T) catalog.CatalogDB { t.Helper() db := setupModuleFilterDB(t) - if _, err := db.Exec(`INSERT INTO microflows VALUES (?, ?, ?, ?, '', 'Microflow', '', '', 0, 0, 0)`, + if _, err := db.Exec(`INSERT INTO microflows VALUES (?, ?, ?, ?, '', 'Microflow', '', '', 0, 0, 0, 0)`, "ModB_mf2", "ModB_Sibling", "ModB.Sibling", "ModB"); err != nil { t.Fatalf("insert sibling microflow: %v", err) } diff --git a/mdl/linter/context_test.go b/mdl/linter/context_test.go index e37ceb1fcd..72ea32cf65 100644 --- a/mdl/linter/context_test.go +++ b/mdl/linter/context_test.go @@ -44,7 +44,7 @@ func setupModuleFilterDB(t *testing.T) catalog.CatalogDB { _, err = db.Exec(`CREATE TABLE microflows ( Id TEXT, Name TEXT, QualifiedName TEXT, ModuleName TEXT, Folder TEXT, MicroflowType TEXT, Description TEXT, ReturnType TEXT, - ParameterCount INTEGER, ActivityCount INTEGER, Complexity INTEGER + ParameterCount INTEGER, ActivityCount INTEGER, TotalActivityCount INTEGER DEFAULT 0, Complexity INTEGER )`) if err != nil { t.Fatalf("create microflows table: %v", err) @@ -63,7 +63,7 @@ func setupModuleFilterDB(t *testing.T) catalog.CatalogDB { mod+"_e", mod+"_Entity", mod+".Entity", mod); err != nil { t.Fatalf("insert entity for %s: %v", mod, err) } - if _, err := db.Exec(`INSERT INTO microflows VALUES (?, ?, ?, ?, '', 'Microflow', '', '', 0, 0, 0)`, + if _, err := db.Exec(`INSERT INTO microflows VALUES (?, ?, ?, ?, '', 'Microflow', '', '', 0, 0, 0, 0)`, mod+"_mf", mod+"_Flow", mod+".Flow", mod); err != nil { t.Fatalf("insert microflow for %s: %v", mod, err) } diff --git a/mdl/linter/rules/empty_test.go b/mdl/linter/rules/empty_test.go index 4b05cb9faa..b19960af61 100644 --- a/mdl/linter/rules/empty_test.go +++ b/mdl/linter/rules/empty_test.go @@ -33,14 +33,16 @@ func setupMicroflowsDB(t *testing.T, rows [][]any) catalog.CatalogDB { _, err = db.Exec(`CREATE TABLE microflows ( Id TEXT, Name TEXT, QualifiedName TEXT, ModuleName TEXT, Folder TEXT, MicroflowType TEXT, Description TEXT, ReturnType TEXT, - ParameterCount INTEGER, ActivityCount INTEGER, Complexity INTEGER + ParameterCount INTEGER, ActivityCount INTEGER, TotalActivityCount INTEGER DEFAULT 0, Complexity INTEGER )`) if err != nil { t.Fatalf("failed to create microflows table: %v", err) } for _, row := range rows { - _, err := db.Exec(`INSERT INTO microflows VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + _, err := db.Exec(`INSERT INTO microflows (Id, Name, QualifiedName, ModuleName, Folder, + MicroflowType, Description, ReturnType, ParameterCount, ActivityCount, Complexity) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, row...) if err != nil { t.Fatalf("failed to insert row: %v", err) diff --git a/mdl/linter/starlark.go b/mdl/linter/starlark.go index 769aa7242e..f8aeefbbb1 100644 --- a/mdl/linter/starlark.go +++ b/mdl/linter/starlark.go @@ -950,7 +950,9 @@ func microflowToStarlark(mf Microflow) starlark.Value { "return_type": starlark.String(mf.ReturnType), "parameter_count": starlark.MakeInt(mf.ParameterCount), "activity_count": starlark.MakeInt(mf.ActivityCount), - "complexity": starlark.MakeInt(mf.Complexity), + // Loop bodies included, at any depth; activity_count counts a loop as one. + "total_activity_count": starlark.MakeInt(mf.TotalActivityCount), + "complexity": starlark.MakeInt(mf.Complexity), // microflows() yields all three flow flavours, so a rule naming the // document in a message or a location must not hardcode "Microflow". // Title case matches the document_type spelling Starlark rules use. diff --git a/mdl/linter/starlark_documented_values_test.go b/mdl/linter/starlark_documented_values_test.go index 36f45864e6..4a3ddd8a7a 100644 --- a/mdl/linter/starlark_documented_values_test.go +++ b/mdl/linter/starlark_documented_values_test.go @@ -118,11 +118,11 @@ func everyKindFixtureDB(t *testing.T) catalog.CatalogDB { `CREATE TABLE microflows ( Id TEXT, Name TEXT, QualifiedName TEXT, ModuleName TEXT, Folder TEXT, MicroflowType TEXT, Description TEXT, ReturnType TEXT, - ParameterCount INTEGER, ActivityCount INTEGER, Complexity INTEGER)`, + ParameterCount INTEGER, ActivityCount INTEGER, TotalActivityCount INTEGER DEFAULT 0, Complexity INTEGER)`, `INSERT INTO microflows VALUES - ('f1','MF','Sales.MF','Sales','','MICROFLOW','','',0,0,1), - ('f2','NF','Sales.NF','Sales','','NANOFLOW','','',0,0,1), - ('f3','RU','Sales.RU','Sales','','RULE','','',0,0,1)`, + ('f1','MF','Sales.MF','Sales','','MICROFLOW','','',0,0,0,1), + ('f2','NF','Sales.NF','Sales','','NANOFLOW','','',0,0,0,1), + ('f3','RU','Sales.RU','Sales','','RULE','','',0,0,0,1)`, } for _, s := range stmts { if _, err := db.Exec(s); err != nil { diff --git a/mdl/linter/starlark_flow_noun_test.go b/mdl/linter/starlark_flow_noun_test.go index 191a244635..53c6287820 100644 --- a/mdl/linter/starlark_flow_noun_test.go +++ b/mdl/linter/starlark_flow_noun_test.go @@ -115,14 +115,14 @@ func flowKindsFixtureDB(t *testing.T) catalog.CatalogDB { `CREATE TABLE microflows ( Id TEXT, Name TEXT, QualifiedName TEXT, ModuleName TEXT, Folder TEXT, MicroflowType TEXT, Description TEXT, ReturnType TEXT, - ParameterCount INTEGER, ActivityCount INTEGER, Complexity INTEGER)`, + ParameterCount INTEGER, ActivityCount INTEGER, TotalActivityCount INTEGER DEFAULT 0, Complexity INTEGER)`, `INSERT INTO microflows VALUES - ('f1','Big_MF','Sales.Big_MF','Sales','','MICROFLOW','','',0,100,50), - ('f2','Big_NF','Sales.Big_NF','Sales','','NANOFLOW','','',0,100,50), - ('f3','Big_RU','Sales.Big_RU','Sales','','RULE','','',0,100,50), - ('f4','ACT_MF','Sales.ACT_MF','Sales','','MICROFLOW','','',0,100,50), - ('f5','ACT_NF','Sales.ACT_NF','Sales','','NANOFLOW','','',0,100,50), - ('f6','ACT_RU','Sales.ACT_RU','Sales','','RULE','','',0,100,50)`, + ('f1','Big_MF','Sales.Big_MF','Sales','','MICROFLOW','','',0,100,100,50), + ('f2','Big_NF','Sales.Big_NF','Sales','','NANOFLOW','','',0,100,100,50), + ('f3','Big_RU','Sales.Big_RU','Sales','','RULE','','',0,100,100,50), + ('f4','ACT_MF','Sales.ACT_MF','Sales','','MICROFLOW','','',0,100,100,50), + ('f5','ACT_NF','Sales.ACT_NF','Sales','','NANOFLOW','','',0,100,100,50), + ('f6','ACT_RU','Sales.ACT_RU','Sales','','RULE','','',0,100,100,50)`, `CREATE TABLE activities ( Id TEXT, Name TEXT, Caption TEXT, ActivityType TEXT, ActionType TEXT, MicroflowId TEXT, MicroflowQualifiedName TEXT, ModuleName TEXT, EntityRef TEXT, diff --git a/mdl/linter/starlark_total_activity_count_test.go b/mdl/linter/starlark_total_activity_count_test.go new file mode 100644 index 0000000000..b067208bf9 --- /dev/null +++ b/mdl/linter/starlark_total_activity_count_test.go @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter_test + +import ( + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// total_activity_count is the catalog's TotalActivityCount (loop bodies +// included, mendixlabs/mxcli#1266) on the struct microflows() yields for every +// flow flavour (ako/mxcli#963). A flow with a loop has total > activity_count; +// the control without a loop has them equal. +func TestStarlarkTotalActivityCount(t *testing.T) { + cat, err := catalog.New() + if err != nil { + t.Fatal(err) + } + defer cat.Close() + db := cat.CatalogDB() + if _, err := db.Exec(`INSERT INTO microflows_data (Id, Name, QualifiedName, ModuleName, MicroflowType, ActivityCount, TotalActivityCount) + VALUES ('mf', 'MF_Loop', 'T.MF_Loop', 'T', 'MICROFLOW', 2, 5), + ('nf', 'NF_Loop', 'T.NF_Loop', 'T', 'NANOFLOW', 1, 3), + ('ru', 'RU_Loop', 'T.RU_Loop', 'T', 'RULE', 1, 2), + ('fl', 'MF_Flat', 'T.MF_Flat', 'T', 'MICROFLOW', 4, 4)`); err != nil { + t.Fatalf("fixture: %v", err) + } + + rule := `RULE_ID = "TEST01" +RULE_NAME = "Total" +DESCRIPTION = "probe" +CATEGORY = "quality" +SEVERITY = "info" + +def check(): + return [violation(message = "%s|%d|%d" % (mf.name, mf.activity_count, mf.total_activity_count)) + for mf in microflows()] +` + path := filepath.Join(t.TempDir(), "total.star") + if err := os.WriteFile(path, []byte(rule), 0o644); err != nil { + t.Fatal(err) + } + r, err := linter.LoadStarlarkRule(path) + if err != nil { + t.Fatalf("LoadStarlarkRule: %v", err) + } + ctx := linter.NewLintContextFromDB(db) + var got []string + for _, v := range r.Check(ctx) { + got = append(got, v.Message) + } + if errs := ctx.QueryErrors(); len(errs) > 0 { + t.Fatalf("query errors: %v", errs) + } + sort.Strings(got) + want := []string{"MF_Flat|4|4", "MF_Loop|2|5", "NF_Loop|1|3", "RU_Loop|1|2"} + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Errorf("got:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) + } +} From d7bbb31ef45548f1050e92241608ca173727d828 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 3 Oct 2026 17:50:12 +0000 Subject: [PATCH 3/3] feat(catalog): add a commit ref kind for commit actions and committing create/change (#963) commit $Order on a loop variable wrote no refs row because refs had no commit kind. A commit action, or a create/change with commit Yes/YesWithoutEvents, now emits FLOW -> ENTITY 'commit', resolved through the same intra-flow variable map as change/delete. It stays out of the analysis graph and the caller kinds. The ref_kind skill test now reads every RefKind constant, so a new kind cannot ship undocumented. Catalog schema version 19. Co-Authored-By: Claude Opus 5.5 --- .../skills/fix-issue/findings/mdl-other.jsonl | 1 + .../skills/mendix/write-lint-rules/SKILL.md | 2 +- CHANGELOG.md | 1 + docs-site/src/internals/catalog-schema.md | 9 ++ mdl/catalog/builder_commit_refs_test.go | 117 ++++++++++++++++++ mdl/catalog/builder_graph.go | 5 + mdl/catalog/builder_references.go | 34 +++++ mdl/catalog/lint_rule_doc_vocabulary_test.go | 33 ++--- mdl/catalog/lint_rule_vocabulary_test.go | 2 +- mdl/catalog/tables.go | 6 +- mdl/executor/cmd_search.go | 3 +- 11 files changed, 195 insertions(+), 18 deletions(-) create mode 100644 mdl/catalog/builder_commit_refs_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index ef6edcd7da..2814f3faac 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -94,3 +94,4 @@ {"area": "mdl/linter", "date": "2026-10-03", "symptom": "mxcli report scores a project lower for lint rules that crash: under v0.24.0, project rules written by a newer mxcli (QUAL004, CUSTOM002 reading .document_noun_title) each produced an error-severity 'Starlark rule error: \"microflow\" struct has no .document_noun_title attribute' that counted 10 points against the project", "cause": "StarlarkRule.Check turned every evaluation error into an ordinary SeverityError violation, indistinguishable from a finding; BuildReport and Summarize counted it, and a configured rule severity was applied to it too", "fix": "ruleFailureViolation marks every failure Violation.RuleFailure; a missing struct attribute (matched on the evaluator message, since starlark flattens NoSuchAttrError via fmt.Errorf) becomes info 'rule needs a newer mxcli ()'; BuildReport splits RuleFailures out before counting and every report format lists them separately; Linter.Run skips the severity override for them; an 'undefined:' load failure gets a newer-mxcli hint", "insight": "The score measures the project, so anything about the tooling has to be partitioned out BEFORE counting, not filtered in the formatter. The control that makes the score assertion meaningful is a working rule's finding that does move the score", "issue": "ako/mxcli#952", "file": "mdl/linter/starlark.go (ruleFailureViolation); mdl/linter/report.go (BuildReport); mdl/linter/linter.go (Run); mdl/linter/report_format.go", "test": "mdl/linter/starlark_rule_failure_test.go"} {"area": "mdl/linter", "date": "2026-10-03", "symptom": "MPR012 (legacy static/dynamic image, CE0582) fires on pages with a native layout (Atlas_Core.NativePhone_Default), where mxbuild 11.13 builds them clean; `check --references` likewise refuses a classic `dropdown` on a native page as MDL-WIDGET40 (CE0582), also clean in mxbuild", "cause": "Both rules assume every page is rendered by the React client. Nothing recorded a layout's platform: ListLayouts left pages.Layout.Native false for every layout, and catalog layouts had only LayoutType, which cannot tell the platforms apart (native uses Default/Popup)", "file": "`mdl/backend/modelsdk/page.go` (`layoutIsNative`), `mdl/catalog/builder_pages.go` (layouts.Platform), `mdl/linter/context_catalog_tables.go` (`NativePages`), `mdl/linter/rules/legacy_image_widget.go`, `mdl/executor/validate_widget_attribute_type.go` (`layoutIsNative`)", "insight": "The platform is the content wrapper's TYPE (Forms$NativeLayoutContent), not a property. The native layouts live in Atlas_Core, a Marketplace module, so the page->layout join must not apply the notPlatformModule filter the iterators use. Sibling check MDL-WIDGET39 (CE2421, textbox on an enumeration) is NOT React-only: measured CE2421 on the native page too, so it keeps firing there", "refs": ["ako/mxcli#953"]} {"area": "mdl/linter", "date": "2026-10-03", "symptom": "MPR002 'Microflow X has no activities' on a microflow or nanoflow whose only content is `return ;` (e.g. a label formatter, `return $currentUser;`)", "cause": "ActivityCount excludes start and end events, so a flow that computes its result in the end event's return value counts 0 activities", "file": "`mdl/linter/rules/empty.go` (`returnsValue`)", "insight": "A non-Void ReturnType is the catalog's witness that the end event returns a value (mxbuild requires it on every end event), so no new column was needed; '' and 'Void' stay reported", "refs": ["ako/mxcli#953"]} +{"area": "mdl/catalog", "date": "2026-10-03", "symptom": "`commit $Order` (on a loop iterator, a parameter or a retrieved list) wrote no refs row; refs_to(entity) could not answer which flows commit an entity", "cause": "refs had no commit ref kind: microflowActionRef / microflowVarActionRef emitted create/change/delete only, and a create/change with commit carried no commit edge", "file": "`mdl/catalog/builder_references.go` (`microflowCommitRef`, `RefKindCommit`)", "insight": "Commit is a use of the entity type like change/delete, resolved through the same intra-flow varEntity map (so the loop-iterator fix of #1266 applies for free), and emitted as a second edge beside create/change rather than replacing them. It stays out of graphRefKinds (would double existing flow->entity edges) and callerRefKinds (a type use, not an invocation). The ref_kind vocabulary test now reads every RefKind constant from the declarations, so a new kind cannot ship undocumented.", "refs": ["ako/mxcli#963", "mendixlabs/mxcli#1266", "mendixlabs/mxcli#1267"]} diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 9914e5cb6e..92defed0dc 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -552,7 +552,7 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific) | `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`, `"ATTRIBUTE"`, `"ENUMERATION"`, `"ENUMERATION_VALUE"`. `LAYOUT`, `WIDGET`, `ATTRIBUTE`, `ENUMERATION` and `ENUMERATION_VALUE` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | | `target_id` | string | Target UUID | | `target_name` | string | `"Sales.Customer"`; three-part for an attribute or an enumeration value: `"Sales.Order.Total"`, `"Sales.OrderStatus.Open"` | -| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"`, `"member"` (binds/reads/writes an attribute or navigates an association), `"xpath"` (an XPath constraint names it), `"type"` (typed as an enumeration), `"value"` (an expression names an enumeration value), `"mapping"` (an import/export mapping maps the entity) — lower-case, unlike the types above. Attribute names used only through a variable in a free-text expression (`$Order/Total`) have no edge | +| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"commit"` (a commit action, or a create/change that commits — beside its `"create"`/`"change"` edge; a commit of a variable whose entity the flow cannot tell has no edge), `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"`, `"member"` (binds/reads/writes an attribute or navigates an association), `"xpath"` (an XPath constraint names it), `"type"` (typed as an enumeration), `"value"` (an expression names an enumeration value), `"mapping"` (an import/export mapping maps the entity) — lower-case, unlike the types above. Attribute names used only through a variable in a free-text expression (`$Order/Total`) have no edge | | `module_name` | string | Source module | ### project_security diff --git a/CHANGELOG.md b/CHANGELOG.md index a2d2244ed2..71340a1522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **A `commit` reference kind in the catalog** (ako/mxcli#963) — `refs` has a `commit` edge from a flow to the entity it commits: a commit action, or a create / change that commits (`Yes` or `YesWithoutEvents`), the latter beside its `create` / `change` edge. The entity of a committed variable is resolved like `change` / `delete`, loop iterators included, so `commit $Order` inside `loop $Order in $Orders` now has a row. `refs_to("M.Order")` in a Starlark rule answers which flows commit an order. `commit` is not in the analysis graph and not a caller kind. The catalog schema version is bumped, so a cached catalog rebuilds. - **`total_activity_count` on the Starlark microflow struct** (ako/mxcli#963) — the catalog's `TotalActivityCount` (loop bodies included, at any depth) for every flow `microflows()` yields: microflows, nanoflows and rules. `activity_count` keeps counting a loop as one activity. - **A project records which mxcli wrote its tooling, and an older binary says so** (ako/mxcli#952) — `mxcli init` and every `init --sync-skills` write `.ai-context/mxcli-tooling.json` (version, build time, date; rewritten only when the version changes). Any command that opens the project with `-p` and a binary **older** than the stamp warns once on stderr, naming both versions and how to update; `init --sync-skills` from an older binary **refuses** instead of rolling the skills, rules and CLAUDE.md back. Releases compare by number, nightlies by tag date, a release against a nightly by build date; dev builds are never reported. Binaries from v0.24.0 and earlier cannot read the stamp, so the regenerated `.claude/bootstrap-mxcli.sh` checks it before choosing a binary: an older `mxcli` on PATH is not linked in (it downloads `MXCLI_TAG` instead), an older `./mxcli` is replaced, and the download lands through a temporary file so a `./mxcli` symlink never has it written through into the PATH binary. - **`init --sync-skills` (alias `--sync`) refreshes the bundled lint rules and the mxcli section of CLAUDE.md / AGENTS.md** (ako/mxcli#952) — it used to refresh only the skills, so a project kept the lint rules and guidance of whichever mxcli first initialised it. Bundled rules are recognised by file name; your own rules beside them are never touched. CLAUDE.md and AGENTS.md are now written between `` / `` markers, and only that section is refreshed — by the sync and by a re-run of `mxcli init` — so project notes outside the markers survive. A file written before the markers is left alone by the sync, with a note; run `mxcli init` once to adopt them. diff --git a/docs-site/src/internals/catalog-schema.md b/docs-site/src/internals/catalog-schema.md index c55e849c25..1dd5655036 100644 --- a/docs-site/src/internals/catalog-schema.md +++ b/docs-site/src/internals/catalog-schema.md @@ -260,6 +260,7 @@ rather than trusting a list here: |---------|------| | `call` | flow calls a microflow / nanoflow / rule / Java action / REST operation | | `create` / `change` / `delete` / `retrieve` | flow acts on an entity object | +| `commit` | flow commits an entity object: a commit action, or a create / change that commits (`Yes` or `YesWithoutEvents`), beside its `create` / `change` edge | | `return` | flow returns an entity type | | `parameter` | page or flow parameter entity type | | `generalize` | entity extends entity | @@ -278,6 +279,14 @@ rather than trusting a list here: | `validate` | attribute validation rule uses a regular expression | | `widget` | page or snippet uses a pluggable / custom widget | +`change`, `delete` and `commit` act on a *variable*, so the entity is resolved +within the flow — from a parameter, a create or retrieve output, or a loop +iterator over one of those. A variable whose entity the flow cannot tell (a +microflow call's result, for one) has no edge. `commit` is not in the analysis +graph: the variable it commits comes from a parameter, create or retrieve that +already links the flow to the entity (or to the association it was retrieved +over), so it would mostly double existing edges. + `schedule`, `publish`, `event` and `settings` are **entry points**: something outside the call graph runs the microflow, so nothing in the model calls it. They are what stops `GRAPH_DEAD_ASSETS`, `LIST CALLERS OF` and lint rule QUAL004 diff --git a/mdl/catalog/builder_commit_refs_test.go b/mdl/catalog/builder_commit_refs_test.go new file mode 100644 index 0000000000..3a7cc8dcac --- /dev/null +++ b/mdl/catalog/builder_commit_refs_test.go @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "sort" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// `commit $Order` on a loop iterator wrote no refs row: refs had no commit kind +// at all (ako/mxcli#963). A flow that commits entity X now has a `commit` edge +// FLOW -> ENTITY, from a commit action and from a create or change that commits. +// A create/change with commit No is the control: it keeps its create/change edge +// and gains no commit edge. +func commitRefsCatalog(t *testing.T) *Catalog { + t.Helper() + const mod = model.ID("mod-t") + orders := listParam("Orders", "T.Order") + order := &domainmodel.Entity{Name: "Order"} + order.ID = "ent-order" + customer := &domainmodel.Entity{Name: "Customer"} + customer.ID = "ent-customer" + dm := &domainmodel.DomainModel{ContainerID: mod, Entities: []*domainmodel.Entity{order, customer}} + dm.ID = "dm-t" + flow := func(id, name string, objs ...microflows.MicroflowObject) *microflows.Microflow { + return µflows.Microflow{ + BaseElement: model.BaseElement{ID: model.ID(id)}, ContainerID: mod, Name: name, + Parameters: []*microflows.MicroflowParameter{orders}, + ObjectCollection: µflows.MicroflowObjectCollection{Objects: objs}, + } + } + + be := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return &model.ProjectSettings{}, nil }, + ListModuleSettingsFunc: func() ([]*types.ModuleSettings, error) { return nil, nil }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{BaseElement: model.BaseElement{ID: mod}, Name: "T"}}, nil + }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + ListRulesFunc: func() ([]*microflows.Rule, error) { return nil, nil }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { return &types.NavigationDocument{}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{ + flow("mf-1", "CommitEach", iterLoop("l1", "Orders", "Order", + newAction("a1", µflows.CommitObjectsAction{CommitVariable: "Order", WithEvents: true}))), + flow("mf-2", "CommitList", + newAction("a2", µflows.CommitObjectsAction{CommitVariable: "$Orders"})), + flow("mf-3", "CreateCommit", + newAction("a3", µflows.CreateObjectAction{EntityQualifiedName: "T.Customer", OutputVariable: "C", Commit: microflows.CommitTypeYes})), + flow("mf-4", "ChangeCommit", iterLoop("l2", "Orders", "Order", + newAction("a4", µflows.ChangeObjectAction{ChangeVariable: "Order", Commit: microflows.CommitTypeYesWithoutEvents}))), + flow("mf-5", "NoCommit", + newAction("a5", µflows.CreateObjectAction{EntityQualifiedName: "T.Customer", OutputVariable: "C", Commit: microflows.CommitTypeNo}), + iterLoop("l3", "Orders", "Order", + newAction("a6", µflows.ChangeObjectAction{ChangeVariable: "Order", Commit: microflows.CommitTypeNo}))), + }, nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { + return []*microflows.Nanoflow{{ + BaseElement: model.BaseElement{ID: "nf-1"}, ContainerID: mod, Name: "NF_CommitEach", + Parameters: []*microflows.MicroflowParameter{orders}, + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{ + iterLoop("l4", "Orders", "Order", + newAction("a7", µflows.CommitObjectsAction{CommitVariable: "Order"})), + }}, + }}, nil + }, + } + cat, err := New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + b := NewBuilder(cat, be) + b.SetFullMode(true) + if err := b.Build(nil); err != nil { + t.Fatalf("catalog build: %v", err) + } + return cat +} + +func TestCommitEmitsCommitRefs(t *testing.T) { + cat := commitRefsCatalog(t) + var got []string + for _, row := range queryRows(t, cat, `SELECT SourceName, TargetName, RefKind FROM refs + WHERE TargetType = 'ENTITY' AND RefKind IN ('create', 'change', 'commit')`) { + got = append(got, row[0].(string)+" "+row[2].(string)+" "+row[1].(string)) + } + sort.Strings(got) + want := []string{ + "T.ChangeCommit change T.Order", + "T.ChangeCommit commit T.Order", + "T.CommitEach commit T.Order", // commit of the loop iterator + "T.CommitList commit T.Order", + "T.CreateCommit commit T.Customer", + "T.CreateCommit create T.Customer", + "T.NF_CommitEach commit T.Order", + // Control: create/change with commit No — no commit edge. + "T.NoCommit change T.Order", + "T.NoCommit create T.Customer", + } + if len(got) != len(want) { + t.Fatalf("refs rows:\n%v\nwant:\n%v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("refs rows:\n%v\nwant:\n%v", got, want) + } + } +} diff --git a/mdl/catalog/builder_graph.go b/mdl/catalog/builder_graph.go index 699a6cc601..12e2a9de01 100644 --- a/mdl/catalog/builder_graph.go +++ b/mdl/catalog/builder_graph.go @@ -18,6 +18,11 @@ import ( var graphRefKinds = []string{ "call", "retrieve", "create", "change", "delete", "associate", "generalize", "parameter", "return", + // Not "commit": its variable is resolved from a parameter, a create or a + // retrieve, each of which already links the flow to the entity (directly, + // or through the association an association retrieve names). Adding it + // would mostly double existing edges' weight and shift communities and + // centrality on every project that commits (ako/mxcli#963). // Entry points: something outside the call graph invokes these, so the // microflow they run is reachable even though nothing in the model calls it. Leaving one out does // not hide it from GRAPH_DEAD_ASSETS — that view asks only whether ANY refs diff --git a/mdl/catalog/builder_references.go b/mdl/catalog/builder_references.go index 710942beb5..0fb24e22a7 100644 --- a/mdl/catalog/builder_references.go +++ b/mdl/catalog/builder_references.go @@ -29,6 +29,7 @@ const ( RefKindMenuItem = "menu_item" // Navigation menu item page reference RefKindChange = "change" // Microflow changes an entity object RefKindDelete = "delete" // Microflow deletes an entity object + RefKindCommit = "commit" // Microflow commits an entity object (commit action, or create/change with commit) RefKindCalculate = "calculate" // Calculated attribute uses a microflow RefKindReturn = "return" // Microflow/nanoflow returns an entity type RefKindSchedule = "schedule" // Scheduled event runs a microflow @@ -267,6 +268,36 @@ func microflowVarActionRef(action microflows.MicroflowAction, varEntity map[stri return "", "", "", false } +// microflowCommitRef resolves the entity an action commits: a commit action's +// variable, or the object of a create or change that commits (Yes or +// YesWithoutEvents). It is a second edge beside the create/change one, so +// "which flows commit entity X" is one refs query — the question a commit in a +// loop or a commit without events starts from (mendixlabs/mxcli#1266, #1267). +// A commit of a variable whose entity is not known in the flow emits nothing, +// as for change and delete (ako/mxcli#963). +func microflowCommitRef(action microflows.MicroflowAction, varEntity map[string]string) (entity string, ok bool) { + commits := func(c microflows.CommitType) bool { + return c == microflows.CommitTypeYes || c == microflows.CommitTypeYesWithoutEvents + } + resolve := func(v string) (string, bool) { + qn, found := varEntity[strings.TrimPrefix(v, "$")] + return qn, found && qn != "" + } + switch a := action.(type) { + case *microflows.CommitObjectsAction: + return resolve(a.CommitVariable) + case *microflows.CreateObjectAction: + if commits(a.Commit) && a.EntityQualifiedName != "" { + return a.EntityQualifiedName, true + } + case *microflows.ChangeObjectAction: + if commits(a.Commit) { + return resolve(a.ChangeVariable) + } + } + return "", false +} + // assocEnds are an association's endpoint entities: From owns the reference // (BSON ParentPointer), To is referenced (ChildPointer). type assocEnds struct{ From, To string } @@ -431,6 +462,9 @@ func (b *Builder) buildReferences() error { if tt, tn, rk, ok := microflowVarActionRef(act.Action, varEntity); ok { emit(tt, tn, rk) } + if qn, ok := microflowCommitRef(act.Action, varEntity); ok { + emit(RefObjectEntity, qn, RefKindCommit) + } } } diff --git a/mdl/catalog/lint_rule_doc_vocabulary_test.go b/mdl/catalog/lint_rule_doc_vocabulary_test.go index f07c6948f0..5b18916137 100644 --- a/mdl/catalog/lint_rule_doc_vocabulary_test.go +++ b/mdl/catalog/lint_rule_doc_vocabulary_test.go @@ -225,26 +225,31 @@ func TestSkillDocumentsRealAttributeDataTypes(t *testing.T) { docRowValues(t, doc, "data_type"), real, "AttributeType.GetTypeName") } -// TestSkillDocumentsRealRefKinds covers ref_kind, which documents no examples -// today. Whatever it documents must be a kind buildReferences emits. +// TestSkillDocumentsRealRefKinds pins the ref_kind row both ways: whatever it +// documents must be a kind buildReferences emits, and every RefKind constant +// must be documented — read from the declarations, so a new kind (commit, +// ako/mxcli#963) cannot ship without a line a rule author can find. func TestSkillDocumentsRealRefKinds(t *testing.T) { doc := lintRuleSkillDoc(t) + declared := constantsWithPrefix(t, "builder_references.go", "RefKind") + // CONTROL: a scan that collected nothing would pass whatever the row said. + if len(declared) < 20 { + t.Fatalf("found only %d RefKind* constants — the scan is broken", len(declared)) + } real := map[string]bool{} - for _, k := range []string{ - RefKindCall, RefKindCreate, RefKindRetrieve, RefKindShowPage, - RefKindGeneralize, RefKindAssociate, RefKindLayout, RefKindDatasource, - RefKindParameter, RefKindAction, RefKindHomePage, RefKindLoginPage, - RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, - RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, - RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, - RefKindMember, RefKindXPath, RefKindType, RefKindValue, RefKindMapping, - } { - real[k] = true + for _, v := range declared { + real[v] = true } - assertDocumentedValuesExist(t, "ref_kind", - docRowValues(t, doc, "ref_kind"), real, "buildReferences") + documented := docRowValues(t, doc, "ref_kind") + assertDocumentedValuesExist(t, "ref_kind", documented, real, "buildReferences") + + for name, v := range declared { + if !slices.Contains(documented, v) { + t.Errorf("%s = %q is emitted but the skill's ref_kind row does not document it", name, v) + } + } } // TestEveryObjectTypeConstantIsInAPublishedList is the other half of the pin. diff --git a/mdl/catalog/lint_rule_vocabulary_test.go b/mdl/catalog/lint_rule_vocabulary_test.go index 61b6445f39..f95d6df118 100644 --- a/mdl/catalog/lint_rule_vocabulary_test.go +++ b/mdl/catalog/lint_rule_vocabulary_test.go @@ -106,7 +106,7 @@ func TestQUAL004EntryKindsAreRealRefKinds(t *testing.T) { RefKindParameter, RefKindAction, RefKindHomePage, RefKindLoginPage, RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, - RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, + RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, RefKindCommit, } { known[k] = true } diff --git a/mdl/catalog/tables.go b/mdl/catalog/tables.go index e89ac53f71..2826edd2e2 100644 --- a/mdl/catalog/tables.go +++ b/mdl/catalog/tables.go @@ -7,6 +7,10 @@ package catalog // // History: // +// 19 (commit refs): refs gains RefKind "commit" (FLOW -> ENTITY) for a +// commit action and a create/change that commits (ako/mxcli#963). No +// column changes, but a cached catalog would keep answering "no flow +// commits X" for every entity until something else rebuilt it. // 18 — layouts_data.Platform ("Web" / "Native"), the content wrapper's // type. LayoutType cannot tell the platforms apart ("Popup" is native), // and MPR012 needs it to stay off native pages, where CE0582 does not @@ -101,7 +105,7 @@ package catalog // SnapshotSource / SourceId / SourceBranch / SourceRevision columns // from every row (issue #576). // 1 — initial flat schema with denormalized snapshot columns on every row. -const CatalogSchemaVersion = "18" +const CatalogSchemaVersion = "19" // MetaSchemaVersion is the catalog_meta key that records the schema version // the cache was built against. diff --git a/mdl/executor/cmd_search.go b/mdl/executor/cmd_search.go index c75c5a64f2..74614a20db 100644 --- a/mdl/executor/cmd_search.go +++ b/mdl/executor/cmd_search.go @@ -35,7 +35,8 @@ import ( // list. // // Deliberately excluded: 'datasource', 'parameter', 'return', 'retrieve', -// 'create', 'change', 'delete', 'associate', 'generalize', 'layout' and 'sync'. +// 'create', 'change', 'delete', 'commit', 'associate', 'generalize', 'layout' +// and 'sync'. // Those are uses of a TYPE or a LAYOUT, not invocations, and folding them in // would make `show callers of ` a synonym for `show references to`. var callerRefKinds = []string{