diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-mcp-create-drops-flow-sides-and-notes.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-mcp-create-drops-flow-sides-and-notes.json new file mode 100644 index 0000000000..8c762dbd52 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-mcp-create-drops-flow-sides-and-notes.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-10-21", "symptom": "A microflow created over --mcp (Studio Pro PED, 11.15.0-rc.4) read back with every sequence flow 1 -> 3: a CASE built with `@anchor(from: bottom, Approved: (from: right), (empty): (from: bottom, to: top))` lost all three anchors and the merge's bottom exit. A script with any `@annotation` failed the create outright: `microflow object type *microflows.Annotation is not yet supported by the MCP backend`.", "cause": "buildFlowDocContent mapped a SequenceFlow to originId/destinationId/caseValue only, so Studio Pro applied its right -> left default; the splice (flowPatchOps) already sent originConnectionSide/destinationConnectionSide. mapObjectTree had no Annotation case and ObjectCollection.AnnotationFlows was never read. The splice's note line used originId/destinationId, the SequenceFlow constructor's keys. PED's AnnotationFlow is an element type that takes origin/destination plus the required originConnectionIndex/destinationConnectionIndex, and refuses the constructor keys with `\"/origin\":\"Expected reference (string), got undefined\"`. That bug was hidden because the note was refused before its line was mapped.", "file": "mdl/backend/mcp/microflow.go (buildFlowDocContent, mapObjectTree, setPEDSides, pedAnnotationFlow, adaptMicroflowSkeleton, CreateMicroflow), mdl/backend/mcp/microflow_mutator.go (flowPatchOps), sdk/microflows/microflows.go (AnnotationFlow.SidesOrDefault)", "insight": "Compare engines, not shapes: run the same script through the modelsdk engine and over --mcp, then diff each flow's (origin, destination, sides) after resolving PED's $id(/objectCollection/objects/N) references to positions. That diff covered all 10 flows (identical after the fix, every one 1 -> 3 before) and settled the bug faster than reading either writer. Read ped_get_schema per element type: 'constructor type' and 'element type' differ in key names (originId vs origin) for what looks like the same flow list. PED's read-back omits a property at its default, so a missing connection index means 0 (Top), not absent. The pre-11.14 constructor cannot type a flow, so notes are refused there: an untyped note line would be read as a SequenceFlow. Within one update PED stores an AnnotationFlow after a SequenceFlow added later, so the splice adds its note lines last; otherwise its index-addressed sets on the new sequence flow would point at the wrong flow. Still open: ALTER insert over MCP on 11.15 fails whenever it sets line/ControlVector on a flow added in the same update (`undefined is not an object (evaluating 'u.$Type')`), with or without a note.", "refs": ["ako/mxcli#1528"]} diff --git a/docs/03-development/PED_MCP_CAPABILITIES.md b/docs/03-development/PED_MCP_CAPABILITIES.md index 37c726b2d6..a880bf7a8d 100644 --- a/docs/03-development/PED_MCP_CAPABILITIES.md +++ b/docs/03-development/PED_MCP_CAPABILITIES.md @@ -572,6 +572,41 @@ behaviour change, not an error. read the `page-gen-common` skill first); page create and ALTER ran clean on both servers. +### Microflow canvas: flow sides and notes (measured live 2026-10-21, 11.15.0-rc.4) + +- **A flow created without sides is drawn right → left.** The `SequenceFlow` + constructor takes `originConnectionSide` / `destinationConnectionSide` + (`Top | Right | Bottom | Left`). The create path sent neither, so every `@anchor`, + CASE outcome anchor and builder default was lost: a CASE built with + `@anchor(from: bottom, Approved: (from: right), (empty): (from: bottom, to: top))` + read back `1 -> 3` on all eight flows. **Fixed:** `buildFlowDocContent` maps the + connection indexes the way the splice already did (`setPEDSides`). +- **A note is a `Microflows$Annotation` object** (`x`, `y`, `caption`, `size`) and + its line a **`Microflows$AnnotationFlow` element, not a constructor.** Its ends + are `origin` / `destination`, not `originId` / `destinationId`, and its sides are + the required `originConnectionIndex` / `destinationConnectionIndex` themselves. + The constructor keys are refused: `"/origin":"Expected reference (string), got + undefined"`. Before the fix, a microflow with a note failed the whole create + (`mapObjectTree` had no case for it). The splice had the same wrong keys too, + hidden because the note was refused before its line was mapped. +- Verified: the same script read back from Studio Pro and from the modelsdk engine + gives **identical sides on all 10 flows**, both note lines included (an unplaced + note `2 -> 0`, a note placed below its activity `0 -> 2`). It also gives the same + note positions, sizes and captions, and `ped_check_errors` is clean on two + successive calls. A read-back leaves out a connection index of 0, its default. +- The pre-11.14 (full) constructor's flows carry no `$Type`, so a note's line could + only be read as a `SequenceFlow`. The backend refuses notes there rather than + risk an execution edge out of a note. +- Within one `ped_update_document`, an `AnnotationFlow` added *before* a + `SequenceFlow` is stored *after* it. So the splice adds its new sequence flows + (and the sets that address them by index) first, and note lines last. +- **Open: ALTER MICROFLOW insert over MCP fails on 11.15.** Setting + `/flows/N/line/originControlVector` on a flow added in the same update fails the + whole update with `undefined is not an object (evaluating 'u.$Type')`. This + happened without any note: a plain one-activity insert failed, and so did a lone + `SequenceFlow` add followed by that set. Setting the flow's whole `line` instead + is accepted but **not applied**: the read-back has no control vectors. + ## Capability gaps (established 11.11, status re-checked each release) These are the *absences* that bound what the backend can do. They are as diff --git a/mdl-examples/bug-tests/mcp-create-flow-sides-and-notes.mdl b/mdl-examples/bug-tests/mcp-create-flow-sides-and-notes.mdl new file mode 100644 index 0000000000..468488bbd8 --- /dev/null +++ b/mdl-examples/bug-tests/mcp-create-flow-sides-and-notes.mdl @@ -0,0 +1,43 @@ +mdl 1; +-- ============================================================================ +-- A microflow created over --mcp keeps its flow sides and its notes +-- ============================================================================ +-- +-- Run it twice: once with the default engine, once against Studio Pro's MCP +-- server (`--mcp http://localhost/mcp --mcp-dial :`). Then compare +-- each flow's connection sides (the default engine's .mpr via +-- `mxcli bson dump`, Studio Pro's document via ped_read_document). They must +-- be the same. +-- +-- Before the fix, the MCP create sent no sides, so Studio Pro drew every flow +-- right -> left (1 -> 3): the three outcome anchors and the merge's bottom exit +-- were lost. Any @annotation failed the create outright. Measured on +-- 11.15.0-rc.4: after the fix, all 10 flows match the default engine, both +-- note lines included (2 -> 0 for the note above, 0 -> 2 for the one placed +-- below). +-- +-- Studio Pro's MCP server has no delete, so the documents stay in the open +-- project after a run. +-- ============================================================================ + +CREATE MODULE BugMcpSides; + +CREATE OR MODIFY ENUMERATION BugMcpSides.Verdict ( + Approved 'Approved', + Rejected 'Rejected' +); + +CREATE MICROFLOW BugMcpSides.MCPS_CaseNotes ($T: BugMcpSides.Verdict) +BEGIN + @anchor(from: bottom, Approved: (from: right), (empty): (from: bottom, to: top)) + CASE $T + WHEN Approved THEN + @annotation 'Approved takes this branch' + LOG INFO NODE 'MCPS' 'approved'; + WHEN Rejected THEN + @annotation(text: 'Below the rejected log', position: (600, 420)) + LOG INFO NODE 'MCPS' 'rejected'; + WHEN (empty) THEN + LOG WARNING NODE 'MCPS' 'no verdict'; + END CASE; +END; diff --git a/mdl/backend/mcp/microflow.go b/mdl/backend/mcp/microflow.go index 6fcbf8a31c..791da254de 100644 --- a/mdl/backend/mcp/microflow.go +++ b/mdl/backend/mcp/microflow.go @@ -39,6 +39,10 @@ func (b *Backend) CreateMicroflow(mf *microflows.Microflow) error { var ops []pedOpEntry if b.microflowConstructorTakesSkeleton() { ops = adaptMicroflowSkeleton(content) + } else if containsNote(content) { + // The full constructor's flows carry no $Type, so a note's line could + // only be read as a SequenceFlow — an execution edge out of the note. + return fmt.Errorf("microflow %q has a note, which Studio Pro's MCP API takes only from the microflow constructor 11.14 introduced; this Studio Pro has the older one — remove the note or run without --mcp", mf.Name) } if err := b.ensureSchema(microflowDocType); err != nil { return err @@ -104,7 +108,8 @@ func (b *Backend) microflowConstructorTakesSkeleton() bool { // - action, returnValue, the split condition and the loop source become `set` // operations on the stored document (/objectCollection/objects/N/…, nesting // through a loop's objectCollection); -// - each flow gets its $Type, the return type flattens to an enum plus +// - each sequence flow gets its $Type (a note's line already has one), the +// return type flattens to an enum plus // returnTypeEntity/returnTypeEnumeration, and returnVariableName becomes an // operation. func adaptMicroflowSkeleton(content map[string]any) []pedOpEntry { @@ -140,8 +145,10 @@ func adaptMicroflowSkeleton(content map[string]any) []pedOpEntry { if !ok { continue } - fm["$Type"] = "Microflows$SequenceFlow" - for _, k := range []string{"originId", "destinationId"} { + if _, typed := fm["$Type"]; !typed { + fm["$Type"] = "Microflows$SequenceFlow" + } + for _, k := range []string{"originId", "destinationId", "origin", "destination"} { if ref, ok := fm[k].(string); ok { fm[k] = shiftObjectRef(ref, len(params)) } @@ -303,6 +310,7 @@ func (b *Backend) buildFlowDocContent(kind, name string, params []*microflows.Mi "originId": fmt.Sprintf("$id(%s)", op), "destinationId": fmt.Sprintf("$id(%s)", dp), } + setPEDSides(pf, f) cv, err := mapCaseValue(f.CaseValue) if err != nil { return nil, fmt.Errorf("%s %q: %w", kind, name, err) @@ -312,6 +320,16 @@ func (b *Backend) buildFlowDocContent(kind, name string, params []*microflows.Mi } flows = append(flows, pf) } + // A note's line shares the flows list, after the sequence flows, as the + // stored document has it. + for _, af := range oc.AnnotationFlows { + op, ok1 := idPath[af.OriginID] + dp, ok2 := idPath[af.DestinationID] + if !ok1 || !ok2 { + return nil, fmt.Errorf("%s %q: a note is attached to an object that is not supported yet", kind, name) + } + flows = append(flows, pedAnnotationFlow(af, fmt.Sprintf("$id(%s)", op), fmt.Sprintf("$id(%s)", dp))) + } } returnTypeName, rtEntity, rtEnum, err := mfDataType(returnType) @@ -416,6 +434,55 @@ func pedMiddlePoint(pt model.Point) map[string]int { return map[string]int{"x": pt.X, "y": pt.Y} } +var pedSides = []string{"Top", "Right", "Bottom", "Left"} + +// setPEDSides names a sequence flow's connection sides on its PED constructor. +// Left out, Studio Pro draws every flow right -> left (measured on +// 11.15.0-rc.4), whatever the builder or @anchor chose. +func setPEDSides(v map[string]any, f *microflows.SequenceFlow) { + if f.OriginConnectionIndex >= 0 && f.OriginConnectionIndex < 4 { + v["originConnectionSide"] = pedSides[f.OriginConnectionIndex] + } + if f.DestinationConnectionIndex >= 0 && f.DestinationConnectionIndex < 4 { + v["destinationConnectionSide"] = pedSides[f.DestinationConnectionIndex] + } +} + +// pedAnnotationFlow maps a note's line onto PED's Microflows$AnnotationFlow. +// Unlike a SequenceFlow it is an element, not a constructor: its ends are +// origin/destination (not originId/destinationId) and its sides are the +// required connection indexes themselves. +func pedAnnotationFlow(af *microflows.AnnotationFlow, origin, destination string) map[string]any { + sides := af.SidesOrDefault() + return map[string]any{ + "$Type": "Microflows$AnnotationFlow", + "origin": origin, + "destination": destination, + "originConnectionIndex": sides.Origin, + "destinationConnectionIndex": sides.Destination, + } +} + +// containsNote reports whether mapped content has a note anywhere on its +// canvas, loop bodies included. +func containsNote(content map[string]any) bool { + var walk func(objs []any) bool + walk = func(objs []any) bool { + for _, o := range objs { + m, _ := o.(map[string]any) + if m["$Type"] == "Microflows$Annotation" { + return true + } + if body, ok := m["objects"].([]any); ok && walk(body) { + return true + } + } + return false + } + objects, _ := content["objects"].([]any) + return walk(objects) +} + // mapObjectTree maps one executor microflow object onto a PED /objects entry, // registering its path in idPath and recursing into loop bodies (whose objects // nest at /objects/M). Positions come from the executor's layout engine. @@ -520,6 +587,16 @@ func (b *Backend) mapObjectTree(o microflows.MicroflowObject, path string, idPat } m["objects"] = body return m, nil + case *microflows.Annotation: + m := map[string]any{ + "$Type": "Microflows$Annotation", + "relativeMiddlePoint": pos, + "caption": obj.Caption, + } + if obj.Size.Width > 0 && obj.Size.Height > 0 { + m["size"] = map[string]int{"width": obj.Size.Width, "height": obj.Size.Height} + } + return m, nil default: return nil, fmt.Errorf("microflow object type %T is not yet supported by the MCP backend", o) } diff --git a/mdl/backend/mcp/microflow_constructor_test.go b/mdl/backend/mcp/microflow_constructor_test.go index 9b5d9cf927..23e253706d 100644 --- a/mdl/backend/mcp/microflow_constructor_test.go +++ b/mdl/backend/mcp/microflow_constructor_test.go @@ -3,6 +3,8 @@ package mcp import ( + "fmt" + "strings" "testing" "github.com/mendixlabs/mxcli/model" @@ -275,3 +277,194 @@ func TestShiftObjectRef(t *testing.T) { } } } + +// notedMicroflow is a parameter, start -> activity -> end with sides that are +// not PED's default right -> left, and two notes on the activity: one below it +// whose line the builder oriented (top of the note, bottom of the activity), and +// one nothing oriented, which is drawn as for a note above its element. +func notedMicroflow(containerID model.ID) *microflows.Microflow { + obj := func(id string, x, y int) microflows.BaseMicroflowObject { + return microflows.BaseMicroflowObject{BaseElement: model.BaseElement{ID: model.ID(id)}, Position: model.Point{X: x, Y: y}} + } + act := µflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: obj("act", 250, 150)}, + Action: µflows.LogMessageAction{ + LogLevel: microflows.LogLevelInfo, LogNodeName: "'Probe'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "hello"}}, + }, + } + below := µflows.Annotation{BaseMicroflowObject: obj("below", 250, 300), Caption: "Explains the log"} + below.Size = model.Size{Width: 220, Height: 60} + above := µflows.Annotation{BaseMicroflowObject: obj("above", 250, 30), Caption: "Unoriented"} + above.Size = model.Size{Width: 200, Height: 50} + return µflows.Microflow{ + ContainerID: containerID, + Name: "ZzNoted", + Parameters: []*microflows.MicroflowParameter{{Name: "N", Type: µflows.IntegerType{}}}, + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{ + µflows.StartEvent{BaseMicroflowObject: obj("start", 100, 150)}, + act, + µflows.EndEvent{BaseMicroflowObject: obj("end", 400, 300)}, + below, above, + }, + Flows: []*microflows.SequenceFlow{ + {OriginID: "start", DestinationID: "act", OriginConnectionIndex: 1, DestinationConnectionIndex: 3}, + {OriginID: "act", DestinationID: "end", OriginConnectionIndex: 2, DestinationConnectionIndex: 0}, + }, + AnnotationFlows: []*microflows.AnnotationFlow{ + {OriginID: "below", DestinationID: "act", Sides: µflows.ConnectionSides{Origin: 0, Destination: 2}}, + {OriginID: "above", DestinationID: "act"}, + }, + }, + ReturnType: µflows.VoidType{}, + } +} + +// createdContent creates mf against a fake Studio Pro whose microflow +// constructor schema is ctorSchema ("" for the full, pre-11.14 shape) and +// returns the ped_create_document content it was sent. +func createdContent(t *testing.T, ctorSchema string, mf func(model.ID) *microflows.Microflow) (map[string]any, *fakePED, error) { + t.Helper() + ped := newFakePED(t, func(name string, _ map[string]any) (string, bool) { + switch name { + case "ped_get_schema": + if ctorSchema != "" { + return ctorSchema, false + } + case "ped_create_document": + return "SUCCESS: Creating documents (1)", false + case "ped_update_document": + return "SUCCESS: All operations have been performed successfully.", false + case "ped_check_errors": + return "No errors found.", false + } + return "{}", false + }) + b := backendFor(ped) + if err := b.Connect(localProject(t)); err != nil { + t.Fatalf("Connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil { + t.Fatalf("GetModuleByName: %v", err) + } + if err := b.CreateMicroflow(mf(mod.ID)); err != nil { + return nil, ped, err + } + create, ok := ped.callByName("ped_create_document") + if !ok { + t.Fatal("no ped_create_document") + } + return create.Args["documents"].([]any)[0].(map[string]any)["documentContent"].(map[string]any), ped, nil +} + +// A flow's sides are the builder's (and @anchor's) to choose. Measured against +// 11.15.0-rc.4: a create that names none gets Studio Pro's right -> left on +// every flow, so a CASE built with `@anchor(from: bottom, …)` read back 1 -> 3 +// throughout. The constructor takes them as originConnectionSide / +// destinationConnectionSide, as the splice already sends them. +func TestCreateMicroflow_FlowSides(t *testing.T) { + for _, shape := range []struct{ name, schema string }{{"skeleton", microflowCtorSchema1114}, {"full", ""}} { + t.Run(shape.name, func(t *testing.T) { + mf := func(id model.ID) *microflows.Microflow { + m := notedMicroflow(id) + m.ObjectCollection.Objects = m.ObjectCollection.Objects[:3] + m.ObjectCollection.AnnotationFlows = nil + return m + } + content, _, err := createdContent(t, shape.schema, mf) + if err != nil { + t.Fatalf("CreateMicroflow: %v", err) + } + flows, _ := content["flows"].([]any) + want := [][2]string{{"Right", "Left"}, {"Bottom", "Top"}} + if len(flows) != len(want) { + t.Fatalf("flows = %v, want %d", flows, len(want)) + } + for i, f := range flows { + fm, _ := f.(map[string]any) + if fm["originConnectionSide"] != want[i][0] || fm["destinationConnectionSide"] != want[i][1] { + t.Errorf("flows[%d] sides = %v -> %v, want %s -> %s", i, fm["originConnectionSide"], fm["destinationConnectionSide"], want[i][0], want[i][1]) + } + } + }) + } +} + +// A note is a canvas object and its line a typed flow; the skeleton constructor +// takes both in the create. Before, the create path mapped neither: an +// Annotation reached mapObjectTree's default case. +func TestCreateMicroflow_Notes(t *testing.T) { + content, ped, err := createdContent(t, microflowCtorSchema1114, notedMicroflow) + if err != nil { + t.Fatalf("CreateMicroflow: %v", err) + } + objects, _ := content["objects"].([]any) + if len(objects) != 5 { + t.Fatalf("objects = %d, want 5 (the parameter is not one of them)", len(objects)) + } + for i, want := range []struct { + caption string + x, y, w, h int + }{{"Explains the log", 250, 300, 220, 60}, {"Unoriented", 250, 30, 200, 50}} { + note, _ := objects[3+i].(map[string]any) + if note["$Type"] != "Microflows$Annotation" || note["caption"] != want.caption { + t.Errorf("objects[%d] = %v, want the note %q", 3+i, note, want.caption) + } + if fmt.Sprint(note["x"], note["y"]) != fmt.Sprint(want.x, want.y) { + t.Errorf("note %q at %v,%v, want %d,%d", want.caption, note["x"], note["y"], want.x, want.y) + } + if size, _ := note["size"].(map[string]any); fmt.Sprint(size["width"], size["height"]) != fmt.Sprint(want.w, want.h) { + t.Errorf("note %q size = %v, want %dx%d", want.caption, note["size"], want.w, want.h) + } + if _, ok := note["relativeMiddlePoint"]; ok { + t.Errorf("note %q still carries relativeMiddlePoint, which the skeleton drops silently", want.caption) + } + } + + flows, _ := content["flows"].([]any) + if len(flows) != 4 { + t.Fatalf("flows = %d, want 2 sequence flows and 2 note lines: %v", len(flows), flows) + } + for i, want := range []struct { + origin string + sides [2]int + }{{"$id(/objects/3)", [2]int{0, 2}}, {"$id(/objects/4)", [2]int{2, 0}}} { + af, _ := flows[2+i].(map[string]any) + if af["$Type"] != "Microflows$AnnotationFlow" { + t.Errorf("flows[%d] $Type = %v, want Microflows$AnnotationFlow", 2+i, af["$Type"]) + } + // References shift past the parameter like a sequence flow's. + if af["origin"] != want.origin || af["destination"] != "$id(/objects/1)" { + t.Errorf("flows[%d] = %v -> %v, want %s -> $id(/objects/1)", 2+i, af["origin"], af["destination"], want.origin) + } + if fmt.Sprint(af["originConnectionIndex"], af["destinationConnectionIndex"]) != fmt.Sprint(want.sides[0], want.sides[1]) { + t.Errorf("flows[%d] sides = %v -> %v, want %d -> %d", 2+i, af["originConnectionIndex"], af["destinationConnectionIndex"], want.sides[0], want.sides[1]) + } + } + + // The caption and size are constructor properties: nothing about a note is + // left for the update. + update, _ := ped.callByName("ped_update_document") + ops, _ := update.Args["operations"].([]any) + for _, o := range ops { + if p, _ := o.(map[string]any)["path"].(string); strings.HasPrefix(p, "/objectCollection/objects/4") || strings.HasPrefix(p, "/objectCollection/objects/5") { + t.Errorf("update touches a note: %v", o) + } + } +} + +// The full constructor's flows carry no $Type, so a note's line could only be +// read as a sequence flow — an execution edge from the note. It is refused, not +// sent. +func TestCreateMicroflow_NotesRefusedOnFullConstructor(t *testing.T) { + _, ped, err := createdContent(t, "", notedMicroflow) + if err == nil || !strings.Contains(err.Error(), "note") { + t.Fatalf("err = %v, want a refusal naming the note", err) + } + if _, ok := ped.callByName("ped_create_document"); ok { + t.Error("a refused create must send nothing") + } +} diff --git a/mdl/backend/mcp/microflow_mutator.go b/mdl/backend/mcp/microflow_mutator.go index 3d637a2b09..730569ce4f 100644 --- a/mdl/backend/mcp/microflow_mutator.go +++ b/mdl/backend/mcp/microflow_mutator.go @@ -280,8 +280,6 @@ func pedVector(s string) map[string]int { return map[string]int{"width": p["x"], "height": p["y"]} } -var pedSides = []string{"Top", "Right", "Bottom", "Left"} - // pedQualifiedFlowType maps the storage $Type of a flow object to the // qualified name PED reports for it, where the two differ (CLAUDE.md, "BSON // Storage Names vs Qualified Names"). @@ -392,21 +390,19 @@ func (b *Backend) flowPatchOps(d *mcpFlowDeps, spliced bson.D) ([]pedOpEntry, er } } } - for j, id := range addedFlows { - fp := fmt.Sprintf("/flows/%d", len(oldFlowIDs)+j) + // Sequence flows are added before note lines. Within one update PED stores + // an AnnotationFlow after a SequenceFlow added later (measured on + // 11.15.0-rc.4), so the sets below address a new sequence flow by the + // right index only when no note line was added ahead of it. + var addedNotes []*microflows.AnnotationFlow + next := len(oldFlowIDs) + for _, id := range addedFlows { if af, ok := d.annotations[id]; ok { - o, err := ref(string(af.OriginID)) - if err != nil { - return nil, err - } - dst, err := ref(string(af.DestinationID)) - if err != nil { - return nil, err - } - ops = append(ops, pedOpEntry{Path: "/flows", Operation: pedOperation{Type: "add", Value: map[string]any{ - "$Type": "Microflows$AnnotationFlow", "originId": o, "destinationId": dst}}}) + addedNotes = append(addedNotes, af) continue } + fp := fmt.Sprintf("/flows/%d", next) + next++ f, ok := d.flows[id] if !ok { return nil, fmt.Errorf("internal: spliced flow %s was not serialized", id) @@ -420,12 +416,7 @@ func (b *Backend) flowPatchOps(d *mcpFlowDeps, spliced bson.D) ([]pedOpEntry, er return nil, err } v := map[string]any{"$Type": "Microflows$SequenceFlow", "originId": o, "destinationId": dst} - if f.OriginConnectionIndex >= 0 && f.OriginConnectionIndex < 4 { - v["originConnectionSide"] = pedSides[f.OriginConnectionIndex] - } - if f.DestinationConnectionIndex >= 0 && f.DestinationConnectionIndex < 4 { - v["destinationConnectionSide"] = pedSides[f.DestinationConnectionIndex] - } + setPEDSides(v, f) cv, err := mapCaseValue(f.CaseValue) if err != nil { return nil, err @@ -444,6 +435,17 @@ func (b *Backend) flowPatchOps(d *mcpFlowDeps, spliced bson.D) ([]pedOpEntry, er set(fp+"/line/destinationControlVector", pedVector(f.DestinationControlVector)) } } + for _, af := range addedNotes { + o, err := ref(string(af.OriginID)) + if err != nil { + return nil, err + } + dst, err := ref(string(af.DestinationID)) + if err != nil { + return nil, err + } + ops = append(ops, pedOpEntry{Path: "/flows", Operation: pedOperation{Type: "add", Value: pedAnnotationFlow(af, o, dst)}}) + } for i := len(oldFlowIDs) - 1; i >= 0; i-- { if !survivingFlow[oldFlowIDs[i]] { diff --git a/mdl/backend/mcp/microflow_mutator_test.go b/mdl/backend/mcp/microflow_mutator_test.go index df389411de..72b3f6825b 100644 --- a/mdl/backend/mcp/microflow_mutator_test.go +++ b/mdl/backend/mcp/microflow_mutator_test.go @@ -166,6 +166,94 @@ func TestFlowPatchOps_InsertAfter(t *testing.T) { } } +// A note inserted with an activity is added as an object and its line as a +// Microflows$AnnotationFlow element, whose ends are origin/destination — not a +// SequenceFlow constructor's originId/destinationId. Measured on 11.15.0-rc.4: +// the old keys fail the whole update with `"/origin":"Expected reference +// (string), got undefined"`. +func TestFlowPatchOps_InsertNote(t *testing.T) { + var sent []any + f := newFakePED(t, func(name string, args map[string]any) (string, bool) { + switch name { + case "ped_read_document": + return `{"results":[{"path":"/objectCollection/objects","result":[` + + `{"$Type":"Microflows$StartEvent","relativeMiddlePoint":{"x":-98,"y":200}},` + + `{"$Type":"Microflows$EndEvent","relativeMiddlePoint":{"x":564,"y":200}},` + + `{"$Type":"Microflows$ActionActivity","relativeMiddlePoint":{"x":214,"y":200}},` + + `{"$Type":"Microflows$ActionActivity","relativeMiddlePoint":{"x":404,"y":200}}]},` + + `{"path":"/flows","result":[{},{},{}]}]}`, false + case "ped_update_document": + sent = append(sent, args["operations"]) + return "SUCCESS: All operations have been performed successfully.", false + case "ped_check_errors": + return "No errors found.", false + } + return "SUCCESS", false + }) + b := &Backend{client: f.connectClient(t)} + m, _ := openFlowMutator(t, b) + frag := logFragment() + note := µflows.Annotation{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + Position: model.Point{X: 360, Y: 100}, + Size: model.Size{Width: 200, Height: 50}, + }, Caption: "Why we log"} + frag.Objects = append(frag.Objects, note) + frag.AnnotationFlows = []*microflows.AnnotationFlow{{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + OriginID: note.ID, DestinationID: frag.Entry, + }} + if err := m.InsertAfter(model.ID(mfUUID("a")), frag); err != nil { + t.Fatal(err) + } + if err := m.Save(); err != nil { + t.Fatalf("save: %v", err) + } + if len(sent) != 1 { + t.Fatalf("want one ped_update_document, got %d", len(sent)) + } + js, _ := json.Marshal(sent[0]) + var ops []pedOpEntry + if err := json.Unmarshal(js, &ops); err != nil { + t.Fatal(err) + } + var noteAdd, line map[string]any + lineAt, lastSequenceOp := -1, -1 + for i, op := range ops { + v, _ := op.Operation.Value.(map[string]any) + switch { + case op.Operation.Type == "add" && v["$Type"] == "Microflows$Annotation": + noteAdd = v + case op.Operation.Type == "add" && v["$Type"] == "Microflows$AnnotationFlow": + line, lineAt = v, i + case strings.HasPrefix(op.Path, "/flows/3"), op.Operation.Type == "add" && v["$Type"] == "Microflows$SequenceFlow": + lastSequenceOp = i + } + } + // PED stores a note line after a sequence flow added later in the same + // update, so the new sequence flow (and the sets addressing it as + // /flows/3) must come first. + if lineAt < lastSequenceOp { + t.Errorf("the note line (op %d) is added before the new sequence flow's ops (last at %d):\n%s", lineAt, lastSequenceOp, js) + } + if noteAdd == nil || noteAdd["caption"] != "Why we log" { + t.Errorf("note add = %v, want the Annotation with its caption in:\n%s", noteAdd, js) + } + if line == nil { + t.Fatalf("no AnnotationFlow add in:\n%s", js) + } + // The activity is the first object added (index 4), the note the second. + if line["origin"] != "$id(/objectCollection/objects/5)" || line["destination"] != "$id(/objectCollection/objects/4)" { + t.Errorf("note line = %v -> %v, want objects/5 -> objects/4", line["origin"], line["destination"]) + } + if _, ok := line["originId"]; ok { + t.Errorf("note line carries originId, which PED's AnnotationFlow does not take: %v", line) + } + if fmt.Sprint(line["originConnectionIndex"], line["destinationConnectionIndex"]) != "2 0" { + t.Errorf("note line sides = %v -> %v, want the unoriented 2 -> 0", line["originConnectionIndex"], line["destinationConnectionIndex"]) + } +} + // PED addresses by index, so a live document that differs from the stored one // must refuse the write before anything is sent. func TestFlowPatchOps_StaleLiveDocumentIsRefused(t *testing.T) { diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index fe9686c0d7..080686bf85 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -499,12 +499,9 @@ func annotationFlowToGen(af *microflows.AnnotationFlow, v flowVersion) element.E // the note's bottom edge, into the element's top. Both indexes were once 0 // (top), which drew the line out of the top of the note and back down // around it. - origin, destination := int32(2), int32(0) - if af.Sides != nil { - origin, destination = int32(af.Sides.Origin), int32(af.Sides.Destination) - } - g.SetOriginConnectionIndex(origin) - g.SetDestinationConnectionIndex(destination) + sides := af.SidesOrDefault() + g.SetOriginConnectionIndex(int32(sides.Origin)) + g.SetDestinationConnectionIndex(int32(sides.Destination)) // An annotation flow is a Flow like a sequence flow, with the same floors: // the Line was written from 10.0 on, six minors before it existed. if v.hasLine() { diff --git a/sdk/microflows/microflows.go b/sdk/microflows/microflows.go index fdda98db46..89f173c526 100644 --- a/sdk/microflows/microflows.go +++ b/sdk/microflows/microflows.go @@ -405,6 +405,16 @@ type AnnotationFlow struct { Sides *ConnectionSides `json:"sides,omitempty"` } +// SidesOrDefault returns Sides, or — when nothing chose them — the drawing for +// a note sitting above its object: out of the note's bottom edge, into the +// object's top. Every writer uses it, so the engines agree on the default. +func (f *AnnotationFlow) SidesOrDefault() ConnectionSides { + if f.Sides != nil { + return *f.Sides + } + return ConnectionSides{Origin: 2, Destination: 0} +} + // ConnectionSides is the pair of connection indexes a flow attaches at, in the // numbering a SequenceFlow uses for an activity — 0 top, 1 right, 2 bottom, // 3 left. Studio Pro numbers a note's sides the same way.