fix: handle missing TaskPage key in workflow activity mutation

mdl/backend/mpr/workflow_mutator.go

@@ -162,14 +162,21 @@ func (m *mprWorkflowMutator) SetActivityProperty(activityRef string, atPos int,
 	case "PAGE":
 		taskPage := dGetDoc(actDoc, "TaskPage")
 		if taskPage != nil {
+			// TaskPage exists and has a value — update the Page field in place.
 			dSet(taskPage, "Page", value)
+			return nil
+		}
+		pageRef := bson.D{
+			{Key: "$ID", Value: bsonutil.NewIDBsonBinary()},
+			{Key: "$Type", Value: "Workflows$PageReference"},
+			{Key: "Page", Value: value},
+		}
+		if dSet(actDoc, "TaskPage", pageRef) {
+			// TaskPage key exists (nil value) — replaced in place via dSet.
 		} else {
-			pageRef := bson.D{
-				{Key: "$ID", Value: bsonutil.NewIDBsonBinary()},
-				{Key: "$Type", Value: "Workflows$PageReference"},
-				{Key: "Page", Value: value},
-			}
-			dSet(actDoc, "TaskPage", pageRef)
+			// TaskPage key absent — append to activity and replace in BSON tree.
+			actDoc = append(actDoc, bson.E{Key: "TaskPage", Value: pageRef})
+			m.replaceActivity(actDoc)
 		}
 		return nil
 
@@ -555,6 +562,53 @@ func (m *mprWorkflowMutator) Save() error {
 // Internal helpers — activity search
 // ---------------------------------------------------------------------------
 
+// replaceActivity replaces an activity document in the workflow's BSON tree
+// by matching on $ID. This is needed when appending new keys to an activity
+// document, because the slice header returned by findActivityByCaption cannot
+// propagate appends back to the parent bson.A.
+func (m *mprWorkflowMutator) replaceActivity(updated bson.D) {
+	actID := extractBinaryIDFromDoc(dGet(updated, "$ID"))
+	if actID == "" {
+		return
+	}
+	flow := dGetDoc(m.rawData, "Flow")
+	if flow == nil {
+		return
+	}
+	replaceActivityRecursive(flow, actID, updated)
+}
+
+func replaceActivityRecursive(flow bson.D, actID string, updated bson.D) bool {
+	activitiesVal := dGet(flow, "Activities")
+	arr := toBsonA(activitiesVal)
+	if len(arr) == 0 {
+		return false
+	}
+	// Skip the int32 type marker at index 0 if present.
+	start := 0
+	if _, ok := arr[0].(int32); ok {
+		start = 1
+	} else if _, ok := arr[0].(int); ok {
+		start = 1
+	}
+	for i := start; i < len(arr); i++ {
+		actDoc, ok := arr[i].(bson.D)
+		if !ok {
+			continue
+		}
+		if extractBinaryIDFromDoc(dGet(actDoc, "$ID")) == actID {
+			arr[i] = updated
+			return true
+		}
+		for _, nestedFlow := range getNestedFlows(actDoc) {
+			if replaceActivityRecursive(nestedFlow, actID, updated) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // findActivityByCaption searches the workflow for an activity matching caption.
 func (m *mprWorkflowMutator) findActivityByCaption(caption string, atPosition int) (bson.D, error) {
 	flow := dGetDoc(m.rawData, "Flow")

mdl/backend/mpr/workflow_mutator_test.go

@@ -1136,8 +1136,7 @@ func TestWorkflowMutator_InsertBoundaryEvent_NoDelay(t *testing.T) {
 // ---------------------------------------------------------------------------
 
 func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) {
-	// Note: When TaskPage key doesn't pre-exist in BSON, dSet silently fails.
-	// The key must be present (even as nil) for PAGE to work on a new activity.
+	// TaskPage key present with nil value — should be replaced with a new PageReference.
 	act := makeWfActivity("Workflows$UserTask", "Review", "task1")
 	act = append(act, bson.E{Key: "TaskPage", Value: nil})
 	m := newMutator(makeWorkflowDoc(act))
@@ -1157,21 +1156,23 @@ func TestWorkflowMutator_SetActivityProperty_Page_New(t *testing.T) {
 }
 
 func TestWorkflowMutator_SetActivityProperty_Page_MissingKey(t *testing.T) {
-	// BUG: dSet silently fails when TaskPage key is absent — pageRef is lost.
+	// Regression test: dSet silently failed when TaskPage key was absent.
+	// Fixed by appending the key to the activity and replacing it in the BSON tree.
 	act := makeWfActivity("Workflows$UserTask", "Review", "task1")
 	// No TaskPage field at all
 	m := newMutator(makeWorkflowDoc(act))
 
-	// No error returned, but the set is silently lost
 	if err := m.SetActivityProperty("Review", 0, "PAGE", "MyModule.TaskPage"); err != nil {
 		t.Fatalf("SetActivityProperty PAGE failed: %v", err)
 	}
 
 	actDoc, _ := m.findActivityByCaption("Review", 0)
 	taskPage := dGetDoc(actDoc, "TaskPage")
-	// This documents the bug: TaskPage is nil because dSet can't create new keys
-	if taskPage != nil {
-		t.Log("BUG FIXED: TaskPage is now set even when key was absent")
+	if taskPage == nil {
+		t.Fatal("TaskPage should be set even when key was absent")
+	}
+	if got := dGetString(taskPage, "Page"); got != "MyModule.TaskPage" {
+		t.Errorf("Page = %q, want MyModule.TaskPage", got)
 	}
 }