diff --git a/frontend/src/generated/core/api.schemas.ts b/frontend/src/generated/core/api.schemas.ts index 4a6ebe54c419..9f5924ee1b1b 100644 --- a/frontend/src/generated/core/api.schemas.ts +++ b/frontend/src/generated/core/api.schemas.ts @@ -2787,9 +2787,32 @@ export interface PatchedFileSystemApi { * Payload for publishing a freeform canvas's React source via the agent. */ export interface PatchedCanvasPublishApi { + /** The complete single-file React source for the canvas. */ code?: string + /** Short description of the change, stored on the appended version history entry. */ prompt?: string + /** Optional new display name for the canvas (rewrites the leaf segment of its path). */ name?: string + /** + * 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. + * @nullable + */ + expected_current_version_id?: string | null +} + +/** + * 409 body for a guarded canvas publish based on a stale version. + */ +export interface CanvasPublishConflictApi { + /** Human-readable description of the conflict and how to recover. */ + detail: string + /** Always "version_conflict". */ + code: string + /** + * The canvas's live currentVersionId at rejection time (null when the canvas has no versions). + * @nullable + */ + current_version_id: string | null } export interface ContextGenerationApi { diff --git a/frontend/src/generated/core/api.ts b/frontend/src/generated/core/api.ts index 98516f99a798..529edd4faf68 100644 --- a/frontend/src/generated/core/api.ts +++ b/frontend/src/generated/core/api.ts @@ -1439,7 +1439,10 @@ export const getDesktopFileSystemCanvasPartialUpdateUrl = (projectId: string, id * 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. */ export const desktopFileSystemCanvasPartialUpdate = async ( projectId: string, diff --git a/frontend/src/generated/core/api.zod.ts b/frontend/src/generated/core/api.zod.ts index 0c78dd3e7876..04503059b989 100644 --- a/frontend/src/generated/core/api.zod.ts +++ b/frontend/src/generated/core/api.zod.ts @@ -8988,13 +8988,28 @@ export const DesktopFileSystemPartialUpdateBody = /* @__PURE__ */ zod.object({ * 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. */ export const DesktopFileSystemCanvasPartialUpdateBody = /* @__PURE__ */ zod .object({ - code: zod.string().optional(), - prompt: zod.string().optional(), - name: zod.string().optional(), + code: zod.string().optional().describe('The complete single-file React source for the canvas.'), + prompt: zod + .string() + .optional() + .describe('Short description of the change, stored on the appended version history entry.'), + name: zod + .string() + .optional() + .describe('Optional new display name for the canvas (rewrites the leaf segment of its path).'), + expected_current_version_id: zod + .string() + .nullish() + .describe( + "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." + ), }) .describe("Payload for publishing a freeform canvas's React source via the agent.") diff --git a/posthog/api/file_system/file_system.py b/posthog/api/file_system/file_system.py index 17cbeea7748e..6a2c0826547a 100644 --- a/posthog/api/file_system/file_system.py +++ b/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( diff --git a/posthog/api/file_system/test/test_canvas_publish.py b/posthog/api/file_system/test/test_canvas_publish.py index 190867fd796a..678e1225c3f6 100644 --- a/posthog/api/file_system/test/test_canvas_publish.py +++ b/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") diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index 0338462a3d5f..2efab485027b 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -13531,6 +13531,21 @@ export namespace Schemas { suggestion_reason: string; } + /** + * 409 body for a guarded canvas publish based on a stale version. + */ + export interface CanvasPublishConflict { + /** Human-readable description of the conflict and how to recover. */ + detail: string; + /** Always "version_conflict". */ + code: string; + /** + * The canvas's live currentVersionId at rejection time (null when the canvas has no versions). + * @nullable + */ + current_version_id: string | null; + } + /** * Supporting evidence */ @@ -43134,9 +43149,17 @@ export namespace Schemas { * Payload for publishing a freeform canvas's React source via the agent. */ export interface PatchedCanvasPublish { + /** The complete single-file React source for the canvas. */ code?: string; + /** Short description of the change, stored on the appended version history entry. */ prompt?: string; + /** Optional new display name for the canvas (rewrites the leaf segment of its path). */ name?: string; + /** + * 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. + * @nullable + */ + expected_current_version_id?: string | null; } /** diff --git a/services/mcp/src/generated/core/api.ts b/services/mcp/src/generated/core/api.ts index 405ecd88d55d..f16abad5b3d3 100644 --- a/services/mcp/src/generated/core/api.ts +++ b/services/mcp/src/generated/core/api.ts @@ -711,7 +711,10 @@ export const DesktopFileSystemRetrieveParams = /* @__PURE__ */ zod.object({ * 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. */ export const DesktopFileSystemCanvasPartialUpdateParams = /* @__PURE__ */ zod.object({ id: zod.string().describe('A UUID string identifying this file system.'), @@ -724,9 +727,21 @@ export const DesktopFileSystemCanvasPartialUpdateParams = /* @__PURE__ */ zod.ob export const DesktopFileSystemCanvasPartialUpdateBody = /* @__PURE__ */ zod .object({ - code: zod.string().optional(), - prompt: zod.string().optional(), - name: zod.string().optional(), + code: zod.string().optional().describe('The complete single-file React source for the canvas.'), + prompt: zod + .string() + .optional() + .describe('Short description of the change, stored on the appended version history entry.'), + name: zod + .string() + .optional() + .describe('Optional new display name for the canvas (rewrites the leaf segment of its path).'), + expected_current_version_id: zod + .string() + .nullish() + .describe( + "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." + ), }) .describe("Payload for publishing a freeform canvas's React source via the agent.") diff --git a/services/mcp/src/tools/generated/core.ts b/services/mcp/src/tools/generated/core.ts index 3db2c1ae1fb0..4f75c3a51080 100644 --- a/services/mcp/src/tools/generated/core.ts +++ b/services/mcp/src/tools/generated/core.ts @@ -54,6 +54,9 @@ const desktopFileSystemCanvasPartialUpdate = (): ToolBase< if (params.name !== undefined) { body['name'] = params.name } + if (params.expected_current_version_id !== undefined) { + body['expected_current_version_id'] = params.expected_current_version_id + } const result = await context.api.request({ method: 'PATCH', path: `/api/projects/${encodeURIComponent(String(projectId))}/desktop_file_system/${encodeURIComponent(String(params.id))}/canvas/`, diff --git a/services/mcp/tests/unit/__snapshots__/tool-schemas/desktop-file-system-canvas-partial-update.json b/services/mcp/tests/unit/__snapshots__/tool-schemas/desktop-file-system-canvas-partial-update.json index da774c4bf518..0b9d05b9963d 100644 --- a/services/mcp/tests/unit/__snapshots__/tool-schemas/desktop-file-system-canvas-partial-update.json +++ b/services/mcp/tests/unit/__snapshots__/tool-schemas/desktop-file-system-canvas-partial-update.json @@ -5,6 +5,17 @@ "description": "The complete single-file React source for the canvas. Replaces the current code wholesale.", "type": "string" }, + "expected_current_version_id": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "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." + }, "id": { "description": "ID of the canvas (desktop \"dashboard\" item) whose code to publish.", "type": "string" @@ -14,6 +25,7 @@ "type": "string" }, "prompt": { + "description": "Short description of the change, stored on the appended version history entry.", "type": "string" } },