diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index a9fc0b59..dc3f0ed1 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -27,6 +27,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | MDL check gives "unexpected token" on valid-looking syntax | Grammar missing rule or token | `mdl/grammar/MDLParser.g4` + `MDLLexer.g4` | Add rule/token, run `make grammar` | | CE7054 "parameters updated" / CE7067 "does not support body entity" after `send rest request` | `addSendRestRequestAction` emitted wrong BSON: all params as query params, BodyVariable set for JSON bodies | `mdl/executor/cmd_microflows_builder_calls.go` → `addSendRestRequestAction` | Look up operation via `fb.restServices`; route path/query params with `buildRestParameterMappings`; suppress BodyVariable for JSON/TEMPLATE/FILE via `shouldSetBodyVariable` | | `CREATE X` returns "already exists — use create or replace to overwrite" but OR REPLACE is not valid for that type | Error message in executor points to wrong keyword | `mdl/executor/cmd__*.go` — find the `NewAlreadyExistsMsg` call | Change hint from `or replace` to `or modify`; verify the AST stmt uses `CreateOrModify` not `CreateOrReplace` | +| `DESCRIBE microflow` puts shared activities inside an `if … then` block — they should appear after `end if;` | Nested guard split inside `traverseFlowUntilMerge` crosses the outer merge boundary | `mdl/executor/cmd_microflows_show_helpers.go` — guard path in `traverseFlowUntilMerge` (~line 854) | Add `if contID != mergeID` guard before the `isMerge` skip-through so the guard continuation never crosses the outer merge | --- diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index ae907ecf..c9d12900 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -357,6 +357,8 @@ end split; `case` values are qualified entity names. The optional `else` branch handles objects that do not match any listed specialization. +**`cast` only stores the output variable.** Studio Pro persists Microflows$CastAction with a single `VariableName` field — the source variable is implicit (the type-split's input). Use `cast $SpecificName;` to give the specialized variable its name. The two-variable form `$Output = cast $Source;` parses but `$Source` is dropped on roundtrip; prefer the single-variable form. + ### LOOP Statements ```mdl diff --git a/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl b/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl new file mode 100644 index 00000000..9c443a5d --- /dev/null +++ b/mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl @@ -0,0 +1,73 @@ +-- ============================================================================ +-- Bug #475: CE0079 on inheritance split when one branch continues +-- ============================================================================ +-- +-- Symptom (before fix): +-- Studio Pro `mx check` reports CE0079 "Activity has no outgoing flow" +-- on the body of a terminating branch in a `split type` whose other +-- branch continues to a follow-up activity. mxcli's +-- `addStructuredInheritanceSplit` took a "no-merge shortcut" when +-- exactly one non-split branch continued: it wired the parent's next +-- statement directly to the continuing branch's tail and skipped the +-- ExclusiveMerge. The terminating branch's tail then had nowhere to +-- converge — Studio Pro flagged its outgoing-flow gap as CE0079. +-- On re-describe the parent's follow-up activity also leaked inside +-- the case body, breaking subsequent roundtrips. +-- +-- After fix: +-- `addStructuredInheritanceSplit` always emits an ExclusiveMerge when +-- any branch continues, mirroring the invariant `addEnumSplit` +-- already enforces. Both terminating and continuing branches converge +-- on the merge; the parent's next statement attaches after the merge. +-- +-- Validation: +-- `mxcli check` parses the script. +-- `mx check` against the resulting MPR reports 0 errors. +-- Roundtrip (describe → exec → describe) preserves the structure +-- byte-for-byte: the post-split log activity stays outside both case +-- bodies. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/475-inheritance-split-continuing-branch-merge.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow BugTest475.MF_TypedDispatch" +-- ============================================================================ + +create module BugTest475; + +create persistent entity BugTest475.Vehicle ( + Plate : string +); +/ + +create persistent entity BugTest475.Car + extends BugTest475.Vehicle ( + Wheels : integer +); +/ + +create persistent entity BugTest475.Boat + extends BugTest475.Vehicle ( + HullLength : decimal +); +/ + +-- One case continues into the post-split log activity; the other case +-- terminates with a `return`. Before the fix this combination tripped +-- CE0079 on the boat case. After the fix the merge converges both tails +-- and the log activity attaches after the merge. +create microflow BugTest475.MF_TypedDispatch ( + $obj : BugTest475.Vehicle +) +returns boolean +begin + split type $obj + case BugTest475.Car + log info node 'BugTest475' 'Dispatching car'; + case BugTest475.Boat + log info node 'BugTest475' 'Dispatching boat'; + return false; + end split; + log info node 'BugTest475' 'Dispatched'; + return true; +end; +/ diff --git a/mdl-examples/bug-tests/528-nested-guard-shared-activities.mdl b/mdl-examples/bug-tests/528-nested-guard-shared-activities.mdl new file mode 100644 index 00000000..5f49c2c7 --- /dev/null +++ b/mdl-examples/bug-tests/528-nested-guard-shared-activities.mdl @@ -0,0 +1,46 @@ +-- Bug test for issue #528: +-- Microflow MDL extraction is faulty — shared activities after an outer +-- if/merge are incorrectly placed inside the then-block when a nested guard +-- split's false path leads directly to the outer ExclusiveMerge. +-- +-- Verifies: +-- 1. When an inner guard (PipelineDate empty check) has its false path going +-- straight to the outer ExclusiveMerge, the activities that follow the +-- merge (change $Deal, DuplicatePressed handling) appear OUTSIDE the +-- outer `if $Pipeline = true then … end if;` block — not inside it. +-- +-- Reported by: jwinckelmann (Studio Pro v10.24.13, mxcli v0.8.0) +-- Root cause: traverseFlowUntilMerge's guard continuation was skipping +-- through the outer ExclusiveMerge and swallowing the shared +-- activities, leaving nothing to emit after `end if;`. + +-- This script cannot be executed directly because it describes a microflow +-- that only exists in the reporter's project. The MDL below is the EXPECTED +-- output from `DESCRIBE MICROFLOW TCUApp.ACT_Deal_SaveAsVRIPipeline` after +-- the fix is applied — shared activities appear after `end if;`: + +-- if $Deal/Pipeline = true then +-- if $Deal/PipelineDate = empty then +-- show message ...; +-- log info ...; +-- return; +-- end if; +-- end if; +-- change $Deal (...); +-- if $Deal/DuplicatePressed then +-- ... +-- end if; +-- +-- The broken output (before fix) placed `change $Deal` INSIDE the outer if: +-- +-- if $Deal/Pipeline = true then +-- if $Deal/PipelineDate = empty then +-- ... +-- return; +-- end if; +-- change $Deal (...); ← wrong: should be outside the outer if +-- if $Deal/DuplicatePressed then +-- ... +-- end if; +-- end if; +-- ← nothing here: shared acts were swallowed diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index a0baf800..fb5d9c96 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -594,9 +594,6 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt fb.endsWithReturn = savedEndsWithReturn if allBranchesReturn { fb.endsWithReturn = true - } else if len(branchTails) == 1 && !branchTails[0].fromSplit { - fb.nextConnectionPoint = branchTails[0].id - fb.nextFlowCase = branchTails[0].caseValue } else if len(branchTails) > 0 { merge := µflows.ExclusiveMerge{ BaseMicroflowObject: microflows.BaseMicroflowObject{ diff --git a/mdl/executor/cmd_microflows_builder_inheritance_split_merge_test.go b/mdl/executor/cmd_microflows_builder_inheritance_split_merge_test.go new file mode 100644 index 00000000..19ae40a5 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_inheritance_split_merge_test.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// TestInheritanceSplitAlwaysEmitsMergeWhenBranchContinues guards against a +// describe/exec roundtrip regression where `addStructuredInheritanceSplit` +// used to take a "no-merge shortcut" when exactly one non-split branch +// continued: it wired the parent's next statement directly to the +// continuing case's tail. Two things broke: +// +// 1. Re-describe emitted the parent's continuation inside the case body +// (visually burying statements in the wrong scope). +// 2. Studio Pro raised CE0079 ("condition value should be configured for +// an outgoing flow") on terminating branches because their cases had +// no merge to converge on. +func TestInheritanceSplitAlwaysEmitsMergeWhenBranchContinues(t *testing.T) { + fb := &flowBuilder{ + spacing: HorizontalSpacing, + measurer: &layoutMeasurer{}, + } + + // An InheritanceSplit with one continuing case (`CastedA`) and a + // terminating else that returns. Before the fix this took the no-merge + // shortcut because branchTails == 1 && !fromSplit. + fb.addStructuredInheritanceSplit(&ast.InheritanceSplitStmt{ + Variable: "obj", + Cases: []ast.InheritanceSplitCase{ + { + Entity: ast.QualifiedName{Module: "M", Name: "CastedA"}, + Body: []ast.MicroflowStatement{ + &ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "continue"}}, + }, + }, + }, + ElseBody: []ast.MicroflowStatement{ + &ast.ReturnStmt{}, + }, + }) + + var merge *microflows.ExclusiveMerge + for _, obj := range fb.objects { + if m, ok := obj.(*microflows.ExclusiveMerge); ok { + merge = m + } + } + if merge == nil { + t.Fatal("expected ExclusiveMerge to be created when one branch continues — no-merge shortcut regression") + } +} diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 2225d058..ff84b6fe 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -881,17 +881,22 @@ func traverseFlowUntilMerge( *lines = append(*lines, indentStr+"end if;") recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1) - // Continue from the false branch (skip through merge if present) + // Continue from the false branch (skip through merge if present). + // Guard: do not cross the outer merge boundary — if the false path + // leads directly to mergeID, stop here so the activities after the + // merge are emitted by continueAfterSplitJoin, not inside this branch. if falseFlow != nil { contID := falseFlow.DestinationID - if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge { - visited[contID] = true - for _, flow := range flowsByOrigin[contID] { - contID = flow.DestinationID - break + if contID != mergeID { + if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge { + visited[contID] = true + for _, flow := range flowsByOrigin[contID] { + contID = flow.DestinationID + break + } } + traverseFlowUntilMerge(ctx, contID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) } - traverseFlowUntilMerge(ctx, contID, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) } } else { if trueFlow != nil { diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index 485e721d..50cc33df 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -1361,3 +1361,101 @@ func TestTraverseFlow_BothBranchesToMerge_NoSwap(t *testing.T) { t.Errorf("expected no negation when both branches go to merge, got:\n%s", output) } } + +// TestTraverseFlow_Issue528_NestedGuardDoesNotSwallowSharedActivities is a +// regression test for issue #528. +// +// Structure (matches TCUApp.ACT_Deal_SaveAsVRIPipeline): +// +// outerSplit (condition: $Pipeline=true) +// [true] → innerGuardSplit (guard: $PipelineDate=empty, same Y, RIGHT→LEFT false path) +// [true=true] → showMsg → innerReturn (EndEvent) +// [false=false] → outerMerge (directly; same Y as innerGuardSplit) +// [false=false] → outerMerge +// ↓ +// sharedAct (change $Deal — must be OUTSIDE the outer if) +// ↓ +// end (EndEvent) +// +// Before the fix the guard continuation in traverseFlowUntilMerge would +// "skip through" outerMerge and swallow sharedAct inside the outer then-block, +// leaving nothing to emit after `end if;`. +func TestTraverseFlow_Issue528_NestedGuardDoesNotSwallowSharedActivities(t *testing.T) { + e := newTestExecutor() + + // inner guard split: same Y as outerSplit so its false path looks like a + // guard continuation (flowLooksLikeGuardContinuation relies on Y equality + // and RIGHT→LEFT connection indices). + innerGuardFalseFlow := mkBranchFlow("inner_guard", "outer_merge", µflows.ExpressionCase{Expression: "false"}) + innerGuardFalseFlow.OriginConnectionIndex = AnchorRight + innerGuardFalseFlow.DestinationConnectionIndex = AnchorLeft + + mkObjAt := func(id string, x, y int) microflows.BaseMicroflowObject { + return microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: mkID(id)}, + Position: model.Point{X: x, Y: y}, + } + } + + activityMap := map[model.ID]microflows.MicroflowObject{ + mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObjAt("start", 0, 60)}, + mkID("outer_split"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObjAt("outer_split", 50, 60), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$Pipeline = true"}, + }, + mkID("inner_guard"): µflows.ExclusiveSplit{ + BaseMicroflowObject: mkObjAt("inner_guard", 150, 60), + SplitCondition: µflows.ExpressionSplitCondition{Expression: "$PipelineDate = empty"}, + }, + mkID("show_msg"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObjAt("show_msg", 150, 160)}, + Action: µflows.LogMessageAction{LogLevel: "Warning", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "date required"}}}, + }, + mkID("inner_return"): µflows.EndEvent{BaseMicroflowObject: mkObjAt("inner_return", 150, 260)}, + mkID("outer_merge"): µflows.ExclusiveMerge{BaseMicroflowObject: mkObjAt("outer_merge", 300, 60)}, + mkID("shared_act"): µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObjAt("shared_act", 400, 60)}, + Action: µflows.CommitObjectsAction{CommitVariable: "Deal"}, + }, + mkID("end"): µflows.EndEvent{BaseMicroflowObject: mkObjAt("end", 500, 60)}, + } + + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("start"): {mkFlow("start", "outer_split")}, + mkID("outer_split"): { + mkBranchFlow("outer_split", "inner_guard", µflows.ExpressionCase{Expression: "true"}), + mkBranchFlow("outer_split", "outer_merge", µflows.ExpressionCase{Expression: "false"}), + }, + mkID("inner_guard"): { + mkBranchFlow("inner_guard", "show_msg", µflows.ExpressionCase{Expression: "true"}), + innerGuardFalseFlow, + }, + mkID("show_msg"): {mkFlow("show_msg", "inner_return")}, + mkID("outer_merge"): {mkFlow("outer_merge", "shared_act")}, + mkID("shared_act"): {mkFlow("shared_act", "end")}, + } + + splitMergeMap := map[model.ID]model.ID{ + mkID("outer_split"): mkID("outer_merge"), + // inner_guard has no merge: its true branch terminates, false goes to outer_merge + } + + var lines []string + visited := map[model.ID]bool{} + e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, splitMergeMap, visited, nil, nil, &lines, 0, nil, 0, nil) + + out := strings.Join(lines, "\n") + + // shared_act must appear AFTER `end if;`, not inside the outer if block. + endIfIdx := strings.Index(out, "end if;") + sharedIdx := strings.Index(out, "commit $Deal;") + if endIfIdx < 0 { + t.Fatalf("expected 'end if;' in output, got:\n%s", out) + } + if sharedIdx < 0 { + t.Fatalf("expected 'commit $Deal;' in output, got:\n%s", out) + } + if sharedIdx <= endIfIdx { + t.Errorf("issue #528: shared activity emitted inside outer if block instead of after end if;\n%s", out) + } +} diff --git a/mdl/executor/validate_microflow_split_collect_test.go b/mdl/executor/validate_microflow_split_collect_test.go new file mode 100644 index 00000000..ab603933 --- /dev/null +++ b/mdl/executor/validate_microflow_split_collect_test.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// flowRefCollector.collectFromStatements must descend into EnumSplitStmt +// case bodies and the else body. A regression in PR #475's first revision +// left the EnumSplitStmt branch with an empty case body in a Go type +// switch, so the loop walking case bodies was silently stolen by the +// next case (InheritanceSplitStmt). The result was that microflow calls +// inside any `case ... when ... then ...` branch escaped reference +// validation. +func TestValidateMicroflowReferences_DescendsIntoEnumSplitCases(t *testing.T) { + moduleID := model.ID("module-1") + backend := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{ + BaseElement: model.BaseElement{ID: moduleID}, + Name: "SyntheticAudit", + }}, nil + }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return nil, nil + }, + } + ctx, _ := newMockCtx(t, withBackend(backend)) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "SyntheticAudit", Name: "RouteByStatus"}, + Body: []ast.MicroflowStatement{ + &ast.EnumSplitStmt{ + Variable: "Status", + Cases: []ast.EnumSplitCase{ + { + Values: []string{"Open"}, + Body: []ast.MicroflowStatement{ + &ast.CallMicroflowStmt{ + MicroflowName: ast.QualifiedName{Module: "SyntheticAudit", Name: "MissingHandler"}, + }, + }, + }, + }, + }, + }, + } + + err := validate(ctx, stmt) + if err == nil { + t.Fatal("expected reference error for microflow inside enum split case body") + } + if !strings.Contains(err.Error(), "microflow not found: SyntheticAudit.MissingHandler") { + t.Fatalf("unexpected error: %v", err) + } +} + +// And the else body of an EnumSplitStmt must also be walked. +func TestValidateMicroflowReferences_DescendsIntoEnumSplitElse(t *testing.T) { + moduleID := model.ID("module-1") + backend := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{ + BaseElement: model.BaseElement{ID: moduleID}, + Name: "SyntheticAudit", + }}, nil + }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return nil, nil + }, + } + ctx, _ := newMockCtx(t, withBackend(backend)) + + stmt := &ast.CreateMicroflowStmt{ + Name: ast.QualifiedName{Module: "SyntheticAudit", Name: "RouteByStatus"}, + Body: []ast.MicroflowStatement{ + &ast.EnumSplitStmt{ + Variable: "Status", + ElseBody: []ast.MicroflowStatement{ + &ast.CallMicroflowStmt{ + MicroflowName: ast.QualifiedName{Module: "SyntheticAudit", Name: "MissingFallback"}, + }, + }, + }, + }, + } + + err := validate(ctx, stmt) + if err == nil { + t.Fatal("expected reference error for microflow inside enum split else body") + } + if !strings.Contains(err.Error(), "microflow not found: SyntheticAudit.MissingFallback") { + t.Fatalf("unexpected error: %v", err) + } +}