fix(library_tools): duplicate legacy library content synchronously to avoid race

xmodule/library_tools.py

@@ -121,7 +121,7 @@ def trigger_duplication(
         ensure_cms("library_content block children may only be duplicated in a CMS context")
         if not isinstance(dest_block, LegacyLibraryContentBlock):
             raise ValueError(f"Can only duplicate children for library_content blocks, not {dest_block.tag} blocks.")
-        if source_block.scope_ids.usage_id.context_key != source_block.scope_ids.usage_id.context_key:
+        if source_block.scope_ids.usage_id.context_key != dest_block.scope_ids.usage_id.context_key:
             raise ValueError(
                 "Cannot duplicate_children across different learning contexts "
                 f"(source={source_block.scope_ids.usage_id}, dest={dest_block.scope_ids.usage_id})"
@@ -131,10 +131,17 @@ def trigger_duplication(
                 "Cannot duplicate_children across different source libraries or versions thereof "
                 f"({source_block.source_library_key=}, {dest_block.source_library_key=})."
             )
-        library_tasks.duplicate_children.delay(
-            user_id=self.user_id,
-            source_block_id=str(source_block.scope_ids.usage_id),
-            dest_block_id=str(dest_block.scope_ids.usage_id),
+        # TODO: This task runs synchronously to avoid a race with duplicate_block's
+        # enclosing store.bulk_operations(), which buffers writes thread-locally.
+        # An out-of-process Celery worker would not see the dest block and would
+        # produce a duplicate with no children. This mirrors the identical fix
+        # applied to sync_from_library in TNL-11339 / #34029. See #36544.
+        library_tasks.duplicate_children.apply(
+            kwargs=dict(
+                user_id=self.user_id,
+                source_block_id=str(source_block.scope_ids.usage_id),
+                dest_block_id=str(dest_block.scope_ids.usage_id),
+            ),
         )
 
     def are_children_syncing(self, library_content_block: LegacyLibraryContentBlock) -> bool:

xmodule/tests/test_library_tools.py

@@ -99,3 +99,80 @@ def test_update_children(self):
         self.tools.trigger_library_sync(content_block, library_version=None)
         content_block = self.store.get_item(content_block.location)
         assert len(content_block.children) == 1
+
+    def _make_library_content_with_child(self):
+        """Build a library + course + populated library_content block."""
+        library = LibraryFactory.create(modulestore=self.store)
+        self.make_block("html", library, data="Hello from the library")
+        course = CourseFactory.create(modulestore=self.store)
+        source_lc = self.make_block(
+            "library_content",
+            course,
+            max_count=1,
+            source_library_id=str(library.location.library_key),
+        )
+        self.tools.trigger_library_sync(source_lc, library_version=None)
+        return course, library, self.store.get_item(source_lc.location)
+
+    def test_unit_trigger_duplication_does_not_enqueue_async_task(self):
+        """
+        Unit test for bug #36544: trigger_duplication must invoke
+        duplicate_children.apply (synchronous), never .delay (async).
+        """
+        course, library, source_lc = self._make_library_content_with_child()
+        dest_lc = self.make_block(
+            "library_content",
+            course,
+            max_count=1,
+            source_library_id=str(library.location.library_key),
+            source_library_version=source_lc.source_library_version,
+        )
+        with mock.patch(
+            "openedx.core.djangoapps.content_libraries.tasks.duplicate_children.delay"
+        ) as mocked_delay, mock.patch(
+            "openedx.core.djangoapps.content_libraries.tasks.duplicate_children.apply"
+        ) as mocked_apply:
+            self.tools.trigger_duplication(source_block=source_lc, dest_block=dest_lc)
+        assert not mocked_delay.called, "must not enqueue an async task; see #36544"
+        assert mocked_apply.called
+
+    def test_integration_trigger_duplication_inside_bulk_operations(self):
+        """
+        Integration test for bug #36544: trigger_duplication must run in-process
+        so its writes land inside the enclosing store.bulk_operations() context,
+        producing a duplicate block whose children are visible immediately.
+        """
+        course, library, source_lc = self._make_library_content_with_child()
+        assert len(source_lc.children) == 1
+
+        with self.store.bulk_operations(course.id):
+            dest_lc = self.make_block(
+                "library_content",
+                course,
+                max_count=1,
+                source_library_id=str(library.location.library_key),
+                source_library_version=source_lc.source_library_version,
+            )
+            self.tools.trigger_duplication(source_block=source_lc, dest_block=dest_lc)
+            dest_lc_reloaded = self.store.get_item(dest_lc.location)
+            assert len(dest_lc_reloaded.children) == 1
+
+    def test_bug_36544_regression_cross_context_guard(self):
+        """
+        Regression for bug #36544: the cross-learning-context guard was a
+        self-comparison (always False) and never fired. Verify that passing
+        source and dest in different courses now raises ValueError.
+        """
+        course_a = CourseFactory.create(modulestore=self.store)
+        course_b = CourseFactory.create(modulestore=self.store)
+        library = LibraryFactory.create(modulestore=self.store)
+        source_lc = self.make_block(
+            "library_content", course_a,
+            source_library_id=str(library.location.library_key),
+        )
+        dest_lc = self.make_block(
+            "library_content", course_b,
+            source_library_id=str(library.location.library_key),
+        )
+        with self.assertRaisesRegex(ValueError, "different learning contexts"):
+            self.tools.trigger_duplication(source_block=source_lc, dest_block=dest_lc)