feat(file-system): optimistic-concurrency guard on canvas publish

posthog/api/file_system/file_system.py

@@ -10,7 +10,7 @@
 from django.db.models import Case, F, IntegerField, Q, QuerySet, Value, When
 from django.db.models.functions import Concat, Lower
 
-from drf_spectacular.utils import extend_schema
+from drf_spectacular.utils import OpenApiResponse, extend_schema
 from rest_framework import filters, pagination, serializers, status, viewsets
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -1032,9 +1032,45 @@ def _retroactively_fix_folders_and_depth(self, user: User) -> None:
 class CanvasPublishSerializer(serializers.Serializer):
     """Payload for publishing a freeform canvas's React source via the agent."""
 
-    code = serializers.CharField(allow_blank=True, trim_whitespace=False)
-    prompt = serializers.CharField(required=False, allow_blank=True, trim_whitespace=False)
-    name = serializers.CharField(required=False, allow_blank=False, trim_whitespace=True)
+    code = serializers.CharField(
+        allow_blank=True,
+        trim_whitespace=False,
+        help_text="The complete single-file React source for the canvas.",
+    )
+    prompt = serializers.CharField(
+        required=False,
+        allow_blank=True,
+        trim_whitespace=False,
+        help_text="Short description of the change, stored on the appended version history entry.",
+    )
+    name = serializers.CharField(
+        required=False,
+        allow_blank=False,
+        trim_whitespace=True,
+        help_text="Optional new display name for the canvas (rewrites the leaf segment of its path).",
+    )
+    expected_current_version_id = serializers.CharField(
+        required=False,
+        allow_null=True,
+        allow_blank=False,
+        help_text=(
+            "Optimistic-concurrency guard: the currentVersionId the publisher based its edits on "
+            "(null when it read a canvas with no versions yet). When provided and the canvas has since "
+            "moved past it (a concurrent publish, or a user's undo) the publish is rejected with a 409 "
+            "version_conflict instead of overwriting the newer head. Omit to publish unguarded."
+        ),
+    )
+
+
+class CanvasPublishConflictSerializer(serializers.Serializer):
+    """409 body for a guarded canvas publish based on a stale version."""
+
+    detail = serializers.CharField(help_text="Human-readable description of the conflict and how to recover.")
+    code = serializers.CharField(help_text='Always "version_conflict".')
+    current_version_id = serializers.CharField(
+        allow_null=True,
+        help_text="The canvas's live currentVersionId at rejection time (null when the canvas has no versions).",
+    )
 
 
 @extend_schema(extensions={"x-product": "core"})
@@ -1100,7 +1136,13 @@ def _get_dashboard_or_400(self) -> FileSystem | Response:
     @extend_schema(
         operation_id="desktop_file_system_canvas_partial_update",
         request=CanvasPublishSerializer,
-        responses={200: FileSystemSerializer},
+        responses={
+            200: FileSystemSerializer,
+            409: OpenApiResponse(
+                response=CanvasPublishConflictSerializer,
+                description="The canvas moved past expected_current_version_id (a concurrent publish or an undo).",
+            ),
+        },
     )
     @action(methods=["PATCH"], detail=True, url_path="canvas")
     def publish_canvas(self, request: Request, *args: Any, **kwargs: Any) -> Response:
@@ -1109,7 +1151,10 @@ def publish_canvas(self, request: Request, *args: Any, **kwargs: Any) -> Respons
         Merges into the dashboard row's `meta` (never replaces it), so existing
         keys like `channelId`/`templateId` survive. Appends a full-file version
         snapshot and points `currentVersionId` at it — the server-side mirror of
-        the app's dashboardsService.saveFreeform.
+        the app's dashboardsService.saveFreeform, including the linear-discard of
+        any redo tail left behind by an undo. When the publisher passes
+        `expected_current_version_id`, a publish based on a stale version is
+        rejected with 409 `version_conflict` instead of overwriting the newer head.
         """
         dashboard = self._get_dashboard_or_400()
         if isinstance(dashboard, Response):
@@ -1120,6 +1165,8 @@ def publish_canvas(self, request: Request, *args: Any, **kwargs: Any) -> Respons
         code = payload.validated_data["code"]
         prompt = payload.validated_data.get("prompt")
         name = payload.validated_data.get("name")
+        has_expected_version = "expected_current_version_id" in payload.validated_data
+        expected_version_id = payload.validated_data.get("expected_current_version_id")
 
         now_ms = int(time.time() * 1000)
         version: dict[str, Any] = {"id": str(uuid4()), "code": code, "createdAt": now_ms}
@@ -1132,12 +1179,39 @@ def publish_canvas(self, request: Request, *args: Any, **kwargs: Any) -> Respons
         with transaction.atomic():
             dashboard = FileSystem.objects.select_for_update().get(pk=dashboard.pk)
             meta = dict(dashboard.meta or {})
+            current_version_id = meta.get("currentVersionId")
+
+            if has_expected_version and current_version_id != expected_version_id:
+                return Response(
+                    {
+                        "detail": "The canvas changed since it was read (a concurrent publish or an undo). "
+                        "Re-fetch the canvas, re-apply the edits to the fresh source, and publish again.",
+                        "code": "version_conflict",
+                        "current_version_id": current_version_id,
+                    },
+                    status=status.HTTP_409_CONFLICT,
+                )
+
             # Snapshot the live author context onto the version (reverting restores it).
             existing_context = meta.get("context")
             if isinstance(existing_context, str):
                 version["context"] = existing_context
             versions = list(meta.get("versions") or [])
             first_publish = not versions and not meta.get("code")
+            # Linear-discard: a publish always becomes the new head, so a redo tail past
+            # the live pointer (left by an undo) is dropped rather than kept as
+            # unreachable history — mirroring the client's undo/redo semantics.
+            if current_version_id:
+                pointer = next(
+                    (
+                        index
+                        for index, existing in enumerate(versions)
+                        if isinstance(existing, dict) and existing.get("id") == current_version_id
+                    ),
+                    None,
+                )
+                if pointer is not None:
+                    versions = versions[: pointer + 1]
             versions.append(version)
 
             meta.update(

posthog/api/file_system/test/test_canvas_publish.py

@@ -6,6 +6,7 @@
 from django.apps import apps
 from django.conf import settings
 
+from parameterized import parameterized
 from rest_framework import status
 
 from posthog.models.file_system.file_system import FileSystem
@@ -72,6 +73,82 @@ def test_publish_canvas_appends_to_existing_history(self):
         self.assertEqual([v["code"] for v in meta["versions"]], ["v1", "v2"])
         self.assertEqual(meta["currentVersionId"], meta["versions"][-1]["id"])
 
+    def _current_version_id(self, item_id: str) -> str:
+        return cast(str, cast(dict, FileSystem.objects.get(id=item_id).meta)["currentVersionId"])
+
+    def test_guarded_publish_with_matching_version_appends(self):
+        item_id = self._create_dashboard()
+        self.client.patch(self._canvas_url(item_id), {"code": "v1"})
+        base = self._current_version_id(item_id)
+
+        response = self.client.patch(
+            self._canvas_url(item_id),
+            {"code": "v2", "expected_current_version_id": base},
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
+        meta = cast(dict, FileSystem.objects.get(id=item_id).meta)
+        self.assertEqual([v["code"] for v in meta["versions"]], ["v1", "v2"])
+
+    def test_guarded_first_publish_with_null_expected_version(self):
+        item_id = self._create_dashboard()
+
+        response = self.client.patch(
+            self._canvas_url(item_id),
+            {"code": "v1", "expected_current_version_id": None},
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
+        meta = cast(dict, FileSystem.objects.get(id=item_id).meta)
+        self.assertEqual([v["code"] for v in meta["versions"]], ["v1"])
+
+    @parameterized.expand(
+        [
+            ("stale_version_id", "not-the-head"),
+            ("null_base_on_published_canvas", None),
+        ]
+    )
+    def test_guarded_publish_conflicts_when_canvas_moved(self, _name: str, expected_version: str | None):
+        item_id = self._create_dashboard()
+        self.client.patch(self._canvas_url(item_id), {"code": "v1"})
+        head = self._current_version_id(item_id)
+
+        response = self.client.patch(
+            self._canvas_url(item_id),
+            {"code": "clobber", "expected_current_version_id": expected_version},
+        )
+        self.assertEqual(response.status_code, status.HTTP_409_CONFLICT, response.json())
+        body = response.json()
+        self.assertEqual(body["code"], "version_conflict")
+        self.assertEqual(body["current_version_id"], head)
+        # The stale publish left the canvas untouched.
+        meta = cast(dict, FileSystem.objects.get(id=item_id).meta)
+        self.assertEqual(meta["code"], "v1")
+        self.assertEqual(meta["currentVersionId"], head)
+        self.assertEqual(len(meta["versions"]), 1)
+
+    def test_publish_after_undo_truncates_redo_tail(self):
+        item_id = self._create_dashboard()
+        self.client.patch(self._canvas_url(item_id), {"code": "v1"})
+        v1 = self._current_version_id(item_id)
+        self.client.patch(self._canvas_url(item_id), {"code": "v2"})
+
+        # The client's undo moves the pointer back without rewriting history.
+        row = FileSystem.objects.get(id=item_id)
+        meta = cast(dict, row.meta)
+        meta["currentVersionId"] = v1
+        meta["code"] = "v1"
+        row.meta = meta
+        row.save(update_fields=["meta"])
+
+        response = self.client.patch(
+            self._canvas_url(item_id),
+            {"code": "v3", "expected_current_version_id": v1},
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
+        meta = cast(dict, FileSystem.objects.get(id=item_id).meta)
+        # The redo tail (v2) is discarded; history is linear again.
+        self.assertEqual([v["code"] for v in meta["versions"]], ["v1", "v3"])
+        self.assertEqual(meta["currentVersionId"], meta["versions"][-1]["id"])
+
     def test_publish_canvas_renames_via_name(self):
         item_id = self._create_dashboard(path="MyChannel/Old name")