diff --git a/posthog/api/comments.py b/posthog/api/comments.py index 29de65754804..5c2e1d239657 100644 --- a/posthog/api/comments.py +++ b/posthog/api/comments.py @@ -1,4 +1,6 @@ +import logging from typing import TYPE_CHECKING, Any, cast +from uuid import UUID from django.core import exceptions as django_exceptions from django.db import transaction @@ -7,6 +9,7 @@ from drf_spectacular.utils import extend_schema from rest_framework import exceptions, pagination, serializers, viewsets +from rest_framework.generics import get_object_or_404 from rest_framework.request import Request from rest_framework.response import Response @@ -25,6 +28,8 @@ if TYPE_CHECKING: from posthog.rbac.user_access_control import UserAccessControl +logger = logging.getLogger(__name__) + def _require_ticket_editor_access( *, team_id: int, item_id: str | None, user_access_control: "UserAccessControl" @@ -48,6 +53,86 @@ def _require_ticket_editor_access( raise exceptions.PermissionDenied("You do not have access to this ticket") +def _record_task_comment_activity( + comment: Comment, + mentions: list[int], + *, + activity_at=None, + include_relationship_recipients: bool = True, +) -> None: + try: + from products.tasks.backend.facade.api import ( # noqa: PLC0415 — keeps the generic comments API decoupled from the tasks product + record_comment_activity, + ) + + owner_id = None + if comment.scope == "desktop_canvas" and comment.item_id: + from products.canvas.backend.comment_access import canvas_owner_id # noqa: PLC0415 + + owner_id = canvas_owner_id(team_id=comment.team_id, canvas_id=comment.item_id) + + record_comment_activity( + team_id=comment.team_id, + comment_id=comment.id, + mentioned_user_ids=mentions, + include_relationship_recipients=include_relationship_recipients, + target_owner_id=owner_id, + activity_at=activity_at, + ) + except Exception: + logger.exception("Failed to project task comment activity", extra={"comment_id": str(comment.id)}) + + +def _mentions_allowed_for_comment_target( + *, team_id: int, scope: str, item_id: str | None, item_context: dict | None +) -> bool: + if scope not in {"task", "task_artifact", "desktop_canvas"}: + return True + task_id = item_id if scope == "task" else (item_context or {}).get("taskId") + if not task_id: + return False + from products.tasks.backend.facade.api import task_comment_mentions_allowed # noqa: PLC0415 + + return task_comment_mentions_allowed(team_id=team_id, task_id=task_id) + + +def _task_comment_target_is_accessible( + *, team_id: int, user_id: int | None, task_id: str, scope: str, item_id: str | None +) -> bool: + from products.tasks.backend.facade.api import task_comment_target_is_accessible # noqa: PLC0415 + + if scope != "desktop_canvas": + return task_comment_target_is_accessible( + team_id=team_id, + user_id=user_id, + task_id=task_id, + scope=scope, + item_id=item_id, + ) + if not task_comment_target_is_accessible( + team_id=team_id, + user_id=user_id, + task_id=task_id, + scope="task", + item_id=task_id, + ): + return False + + from products.canvas.backend.comment_access import canvas_belongs_to_task # noqa: PLC0415 + + try: + parsed_task_id = UUID(task_id) + except ValueError: + return False + if not item_id: + return False + return canvas_belongs_to_task( + team_id=team_id, + canvas_id=item_id, + task_id=parsed_task_id, + ) + + class CommentSerializer(serializers.ModelSerializer): def _extract_mentions_from_rich_content(self, rich_content: dict | None) -> list[int]: if not rich_content: @@ -71,7 +156,8 @@ def find_mentions(node: Any) -> None: find_mentions(rich_content) return mentions - created_by = UserBasicSerializer(read_only=True) + created_by = UserBasicSerializer(read_only=True, allow_null=True) + item_context = serializers.JSONField(required=False, allow_null=True) deleted = ClassicBehaviorBooleanFieldSerializer() mentions = serializers.ListField(child=serializers.IntegerField(), write_only=True, required=False) slug = serializers.CharField(write_only=True, required=False) @@ -128,6 +214,19 @@ def validate(self, data): if "is_task" in data and bool(data["is_task"]) != bool(instance.is_task): raise exceptions.ValidationError({"is_task": "Cannot change task state after creation."}) + if not instance and (parent := data.get("source_comment")): + root = parent.source_comment or parent + if root.team_id != self.context["get_team"]().id: + raise exceptions.ValidationError({"source_comment": "Comment not found."}) + data["source_comment"] = root + data["scope"] = root.scope + data["item_id"] = root.item_id + reply_context = data.get("item_context") or {} + data["item_context"] = { + **(root.item_context or {}), + **({"is_emoji": reply_context["is_emoji"]} if "is_emoji" in reply_context else {}), + } + # Check both the comment's persisted (scope, item_id) and the submitted target — so losing # ticket editor access after creation, and re-scoping a comment into or out of a ticket, # are all caught, not just fresh ticket-message creation. @@ -147,6 +246,20 @@ def validate(self, data): user_access_control=self.context["get_user_access_control"](), ) + target_scope = data.get("scope", instance.scope if instance else None) + target_item_id = data.get("item_id", instance.item_id if instance else None) + target_context = data.get("item_context", instance.item_context if instance else None) or {} + if target_scope in {"task", "task_artifact", "desktop_canvas"}: + task_id = target_item_id if target_scope == "task" else target_context.get("taskId") + if not _task_comment_target_is_accessible( + team_id=self.context["get_team"]().id, + user_id=request.user.id, + task_id=task_id or "", + scope=target_scope, + item_id=target_item_id, + ): + raise exceptions.PermissionDenied("You do not have access to this task comment target") + # Skip content validation when soft-deleting a comment is_deleting = data.get("deleted") is True if not is_deleting: @@ -188,6 +301,13 @@ def create(self, validated_data: Any) -> Any: validated_data["team_id"] = self.context["team_id"] mentions = self._filter_mentions_to_organization(mentions, self.context["get_organization"]().id) + if not _mentions_allowed_for_comment_target( + team_id=self.context["team_id"], + scope=validated_data["scope"], + item_id=validated_data.get("item_id"), + item_context=validated_data.get("item_context"), + ): + mentions = [] comment = super().create(validated_data) @@ -195,6 +315,7 @@ def create(self, validated_data: Any) -> Any: send_discussions_mentioned.delay(comment.id, mentions, slug) produce_discussion_mention_events(comment, mentions, slug) send_mention_notifications(comment, mentions, slug) + _record_task_comment_activity(comment, mentions) return comment @@ -208,6 +329,13 @@ def update(self, instance: Comment, validated_data: dict, **kwargs: Any) -> Comm request = self.context["request"] mentions = self._filter_mentions_to_organization(mentions, self.context["get_organization"]().id) + if not _mentions_allowed_for_comment_target( + team_id=instance.team_id, + scope=validated_data.get("scope", instance.scope), + item_id=validated_data.get("item_id", instance.item_id), + item_context=validated_data.get("item_context", instance.item_context), + ): + mentions = [] with transaction.atomic(): locked_instance = Comment.objects.select_for_update().get(pk=instance.pk) @@ -225,6 +353,12 @@ def update(self, instance: Comment, validated_data: dict, **kwargs: Any) -> Comm send_discussions_mentioned.delay(updated_instance.id, mentions, slug) produce_discussion_mention_events(updated_instance, mentions, slug) send_mention_notifications(updated_instance, mentions, slug) + _record_task_comment_activity( + updated_instance, + mentions, + activity_at=timezone.now(), + include_relationship_recipients=False, + ) return updated_instance @@ -240,6 +374,9 @@ class CommentListQueryParamsSerializer(serializers.Serializer): help_text="Filter by resource type (e.g. Dashboard, FeatureFlag, Insight, Replay).", ) item_id = serializers.CharField(required=False, help_text="Filter by the ID of the resource being commented on.") + task_id = serializers.UUIDField( + required=False, help_text="Owning task for task, task_artifact, and desktop_canvas comment scopes." + ) search = serializers.CharField(required=False, help_text="Full-text search within comment content.") source_comment = serializers.CharField(required=False, help_text="Filter replies to a specific parent comment.") kind = serializers.ChoiceField( @@ -277,6 +414,22 @@ def get_serializer_context(self) -> dict[str, Any]: context["get_user_access_control"] = lambda: self.user_access_control return context + def safely_get_object(self, queryset: QuerySet) -> Comment: + lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field + lookup_value = self.kwargs[lookup_url_kwarg] + comment = get_object_or_404(queryset, **{self.lookup_field: lookup_value}) + if comment.scope in {"task", "task_artifact", "desktop_canvas"}: + task_id = comment.item_id if comment.scope == "task" else (comment.item_context or {}).get("taskId") + if not _task_comment_target_is_accessible( + team_id=self.team_id, + user_id=self.request.user.id, + task_id=task_id or "", + scope=comment.scope, + item_id=comment.item_id, + ): + raise exceptions.NotFound() + return comment + def _filter_ticket_scoped_queryset(self, queryset: QuerySet, item_id: str | None) -> QuerySet: """conversations_ticket comments are ticket messages — restrict them to tickets the caller has viewer access to, mirroring TicketViewSet's own object-level filtering.""" @@ -320,10 +473,23 @@ def safely_get_queryset(self, queryset: QuerySet) -> QuerySet: queryset = queryset.filter(scope=scope) if scope == "conversations_ticket": queryset = self._filter_ticket_scoped_queryset(queryset, params.get("item_id")) + elif scope in {"task", "task_artifact", "desktop_canvas"}: + task_id = params.get("task_id") + item_id = params.get("item_id") + if not _task_comment_target_is_accessible( + team_id=self.team_id, + user_id=self.request.user.id, + task_id=task_id or "", + scope=scope, + item_id=item_id, + ): + return queryset.none() + if scope != "task": + queryset = queryset.filter(item_context__taskId=str(task_id)) else: - # Exclude conversations_ticket comments by default - they use rich content - # from SupportEditor and should only be viewed in the conversations product - queryset = queryset.exclude(scope="conversations_ticket") + # Product-owned scopes require their own object-level access checks and must + # never leak through an unscoped generic comments query. + queryset = queryset.exclude(scope__in=["conversations_ticket", "task", "task_artifact", "desktop_canvas"]) if params.get("item_id"): queryset = queryset.filter(item_id=params.get("item_id")) diff --git a/posthog/api/test/test_comments.py b/posthog/api/test/test_comments.py index 6c342279afca..18bd1a9a8a13 100644 --- a/posthog/api/test/test_comments.py +++ b/posthog/api/test/test_comments.py @@ -1,9 +1,12 @@ +from datetime import timedelta from typing import Any from posthog.test.base import APIBaseTest, QueryMatchingTest from unittest import mock +from django.apps import apps from django.conf import settings +from django.utils import timezone from parameterized import parameterized from rest_framework import status @@ -12,6 +15,8 @@ from posthog.models.activity_logging.activity_log import ActivityLog from posthog.models.comment import Comment from posthog.models.comment.utils import build_comment_item_url, extract_plain_text_from_rich_content +from posthog.models.oauth import OAuthAccessToken, OAuthApplication +from posthog.temporal.oauth import ARRAY_APP_CLIENT_ID_DEV, POSTHOG_AI_APP_CLIENT_ID_DEV from products.conversations.backend.models import Ticket from products.conversations.backend.models.constants import Channel, Status @@ -20,6 +25,507 @@ class TestComments(APIBaseTest, QueryMatchingTest): + def _sandbox_task_comment_client( + self, task_id=None, *, client_id=ARRAY_APP_CLIENT_ID_DEV, scopes="task:read comment:read" + ): + app = OAuthApplication.objects.create( + name="Task comments sandbox", + client_id=client_id, + client_type=OAuthApplication.CLIENT_CONFIDENTIAL, + authorization_grant_type=OAuthApplication.GRANT_AUTHORIZATION_CODE, + redirect_uris="https://example.com/callback", + algorithm="RS256", + organization=self.organization, + user=self.user, + ) + token = OAuthAccessToken.objects.create( + user=self.user, + application=app, + token="pha_task_comments", + scope=scopes, + expires=timezone.now() + timedelta(hours=1), + scoped_teams=[self.team.id], + sandbox_task_id=task_id, + ) + self.client.logout() + self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {token.token}") + return self.client + + def _task_artifact_target(self, *, public: bool = True, creator=None): + task_channel_model = apps.get_model("tasks", "Channel") + task_model = apps.get_model("tasks", "Task") + task_run_model = apps.get_model("tasks", "TaskRun") + channel = None + if public: + channel, _ = task_channel_model.objects.unscoped().get_or_create( + team=self.team, + name="comment-test", + defaults={"created_by": self.user}, + ) + task = task_model.objects.create( + team=self.team, + title="Comment target", + created_by=creator or self.user, + channel=channel, + ) + task_run_model.objects.create( + team=self.team, + task=task, + artifacts=[{"id": "artifact-1", "name": "report.md", "type": "output"}], + ) + return task + + def test_task_artifact_comments_require_a_visible_owning_task(self) -> None: + task = self._task_artifact_target() + payload: dict[str, Any] = { + "content": "Review this", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + } + + created = self.client.post(f"/api/projects/{self.team.id}/comments", payload) + assert created.status_code == status.HTTP_201_CREATED + without_task = self.client.get(f"/api/projects/{self.team.id}/comments?scope=task_artifact&item_id=artifact-1") + assert without_task.json()["results"] == [] + unscoped = self.client.get(f"/api/projects/{self.team.id}/comments?item_id=artifact-1") + assert unscoped.json()["results"] == [] + with_task = self.client.get( + f"/api/projects/{self.team.id}/comments?scope=task_artifact&item_id=artifact-1&task_id={task.id}" + ) + assert [row["id"] for row in with_task.json()["results"]] == [created.json()["id"]] + + def test_task_comments_list_artifacts_comments_and_one_comment(self) -> None: + task = self._task_artifact_target() + task_run_model = apps.get_model("tasks", "TaskRun") + task_run_model.objects.create( + team=self.team, + task=task, + artifacts=[{"id": "artifact-1", "name": "latest-report.md", "type": "output"}], + ) + canvas_id = "019fcbe9-839f-7571-ad42-31aa5f615112" + apps.get_model("tasks", "TaskThreadMessage").objects.for_team(self.team.id).create( + team=self.team, + task=task, + event="canvas_created", + content="Canvas created", + payload={ + "canvas_name": "Research canvas", + "canvas_url": f"https://app.posthog.com/code/canvas/channel/{canvas_id}", + }, + ) + root = Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task_artifact", + item_id="artifact-1", + item_context={ + "taskId": str(task.id), + "anchor": {"kind": "text", "quote": "important output", "start": 0, "end": 16}, + "canvasVersionId": "version-2", + }, + content="Please tighten this section", + ) + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task_artifact", + item_id="artifact-1", + item_context={"taskId": str(task.id), "anchor": {"kind": "document"}}, + source_comment=root, + content="Done", + ) + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task_artifact", + item_id="artifact-1", + item_context={"taskId": str(task.id), "threadState": "unexpected"}, + source_comment=root, + content="Malformed state is still a reply", + ) + client = self._sandbox_task_comment_client(task.id) + + artifacts = client.get(f"/api/projects/{self.team.id}/tasks/{task.id}/artifacts/") + assert artifacts.status_code == status.HTTP_200_OK + assert artifacts.json() == { + "artifacts": [ + { + "id": "artifact-1", + "type": "artifact", + "name": "latest-report.md", + }, + {"id": canvas_id, "type": "canvas", "name": "Research canvas"}, + ] + } + + comments = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/comments/?artifact_id=artifact-1", + ) + assert comments.status_code == status.HTTP_200_OK + assert comments.json()["comments"] == [ + { + "id": str(root.id), + "target": {"id": "artifact-1", "type": "artifact", "name": "latest-report.md"}, + "content": "Please tighten this section", + "selected_text": "important output", + "created_at": root.created_at.isoformat().replace("+00:00", "Z"), + "reply_count": 2, + "resolved": False, + } + ] + + detail = client.get(f"/api/projects/{self.team.id}/tasks/{task.id}/comments/{root.id}/") + assert detail.status_code == status.HTTP_200_OK + assert [comment["content"] for comment in detail.json()["comments"]] == [ + "Please tighten this section", + "Done", + "Malformed state is still a reply", + ] + assert detail.json()["comments"][0]["anchor"] == { + "kind": "text", + "quote": "important output", + } + assert detail.json()["comments"][0]["canvas_version_id"] == "version-2" + assert detail.json()["next"] is None + + def test_task_comments_use_an_opaque_cursor(self) -> None: + task = self._task_artifact_target() + for content in ("First", "Second"): + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(task.id), + content=content, + ) + client = self._sandbox_task_comment_client(task.id) + + first = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/comments/?limit=1", + ).json() + second = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/comments/?limit=1&cursor={first['next']}", + ).json() + + assert [row["content"] for row in first["comments"]] == ["Second"] + assert [row["content"] for row in second["comments"]] == ["First"] + assert second["next"] is None + + def test_task_comments_scan_past_resolved_roots(self) -> None: + task = self._task_artifact_target() + roots = [] + for content in ("Open older", "Resolved newer"): + roots.append( + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(task.id), + content=content, + ) + ) + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(task.id), + source_comment=roots[1], + item_context={"threadState": "resolved"}, + content="resolved", + ) + client = self._sandbox_task_comment_client(task.id) + + response = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/comments/?limit=1", + ).json() + + assert [row["content"] for row in response["comments"]] == ["Open older"] + + def test_task_comment_replies_are_paginated(self) -> None: + task = self._task_artifact_target() + root = Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(task.id), + content="Root", + ) + Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(task.id), + source_comment=root, + content="Reply", + ) + client = self._sandbox_task_comment_client(task.id) + + first = client.get(f"/api/projects/{self.team.id}/tasks/{task.id}/comments/{root.id}/?limit=1").json() + second = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/comments/{root.id}/?limit=1&cursor={first['next']}", + ).json() + + assert [row["content"] for row in first["comments"]] == ["Root"] + assert [row["content"] for row in second["comments"]] == ["Reply"] + assert second["next"] is None + + def test_task_comments_cannot_read_another_task_comment(self) -> None: + current_task = self._task_artifact_target() + other_task = self._task_artifact_target() + other_comment = Comment.objects.create( + team=self.team, + created_by=self.user, + scope="task", + item_id=str(other_task.id), + item_context={"anchor": {"kind": "document"}}, + content="Other task comment", + ) + client = self._sandbox_task_comment_client(current_task.id) + + response = client.get( + f"/api/projects/{self.team.id}/tasks/{current_task.id}/comments/{other_comment.id}/", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_task_comments_require_the_sandbox_task_binding(self) -> None: + task = self._task_artifact_target() + + response = self.client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/artifacts/", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_task_comments_ask_legacy_sandboxes_to_restart(self) -> None: + task = self._task_artifact_target() + client = self._sandbox_task_comment_client() + + response = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/artifacts/", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert "Restart the task" in str(response.json()) + + def test_task_comments_reject_an_alternate_task_url_for_the_same_user(self) -> None: + bound_task = self._task_artifact_target() + other_task = self._task_artifact_target() + client = self._sandbox_task_comment_client(bound_task.id) + + response = client.get( + f"/api/projects/{self.team.id}/tasks/{other_task.id}/artifacts/", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_task_comments_reject_a_posthog_ai_sandbox_token(self) -> None: + task = self._task_artifact_target() + client = self._sandbox_task_comment_client(task.id, client_id=POSTHOG_AI_APP_CLIENT_ID_DEV) + + response = client.get( + f"/api/projects/{self.team.id}/tasks/{task.id}/artifacts/", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_task_comments_require_comment_read_scope(self) -> None: + task = self._task_artifact_target() + client = self._sandbox_task_comment_client(task.id, scopes="task:read") + + response = client.get(f"/api/projects/{self.team.id}/tasks/{task.id}/comments/") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_task_artifact_comments_reject_mismatched_and_private_targets(self) -> None: + other = User.objects.create_and_join(self.organization, "private-task-owner@posthog.com", "password") + task = self._task_artifact_target(public=False, creator=other) + payload: dict[str, Any] = { + "content": "Should not land", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + } + assert self.client.post(f"/api/projects/{self.team.id}/comments", payload).status_code == 403 + + visible_task = self._task_artifact_target() + payload["item_context"]["taskId"] = str(visible_task.id) + payload["item_id"] = "not-on-visible-task" + assert self.client.post(f"/api/projects/{self.team.id}/comments", payload).status_code == 403 + + def test_canvas_comments_use_the_relational_canvas_owner(self) -> None: + task = self._task_artifact_target() + channel = task.channel + canvas_model = apps.get_model("canvas", "Canvas") + canvas = canvas_model.objects.unscoped().create( + team=self.team, + channel=channel, + name="Launch canvas", + created_by=self.user, + ) + canvas_version_model = apps.get_model("canvas", "CanvasSourceVersion") + canvas_version_model.objects.unscoped().create( + team=self.team, + canvas=canvas, + source_hash="a" * 64, + source_object_key="canvases/test/source.json", + source_size=2, + task_id=task.id, + created_by=self.user, + ) + mentioned = User.objects.create_and_join(self.organization, "canvas-mentioned@posthog.com", "password") + payload: dict[str, Any] = { + "content": "Review this canvas", + "scope": "desktop_canvas", + "item_id": str(canvas.id), + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + "mentions": [mentioned.id], + } + + created = self.client.post(f"/api/projects/{self.team.id}/comments", payload) + + assert created.status_code == status.HTTP_201_CREATED + with_task = self.client.get( + f"/api/projects/{self.team.id}/comments?scope=desktop_canvas&item_id={canvas.id}&task_id={task.id}" + ) + assert [row["id"] for row in with_task.json()["results"]] == [created.json()["id"]] + task_activity_model = apps.get_model("tasks", "TaskCommentActivity") + assert ( + task_activity_model.objects.unscoped() + .filter( + team=self.team, + user=mentioned, + task=task, + comment_id=created.json()["id"], + ) + .exists() + ) + + other_task = self._task_artifact_target() + payload["item_context"]["taskId"] = str(other_task.id) + assert self.client.post(f"/api/projects/{self.team.id}/comments", payload).status_code == 403 + + def test_comment_without_a_mention_notifies_the_task_owner(self) -> None: + task = self._task_artifact_target() + owner = User.objects.create_and_join(self.organization, "owner@posthog.com", "password") + task.created_by = owner + task.save(update_fields=["created_by"]) + + created = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "Review when ready", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + }, + ) + + assert created.status_code == status.HTTP_201_CREATED + activity_model = apps.get_model("tasks", "TaskCommentActivity") + activity = activity_model.objects.unscoped().get(team=self.team, user=owner, comment_id=created.json()["id"]) + assert activity.kind == "owned_item_comment" + + @mock.patch("products.tasks.backend.facade.api.record_comment_activity", side_effect=RuntimeError("activity down")) + def test_activity_projection_failure_does_not_fail_comment_creation(self, _record_activity: mock.Mock) -> None: + task = self._task_artifact_target() + + response = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "Still persist this", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + }, + ) + + assert response.status_code == status.HTTP_201_CREATED + assert Comment.objects.filter(id=response.json()["id"], team=self.team).exists() + + def test_reply_inherits_its_root_comment_target(self) -> None: + task = self._task_artifact_target() + root = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "Root", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + }, + ).json() + + response = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "Reply", + "scope": "Insight", + "item_id": "another-resource", + "item_context": {"taskId": "00000000-0000-4000-8000-000000000000", "is_emoji": True}, + "source_comment": root["id"], + }, + ) + + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["source_comment"] == root["id"] + assert response.json()["scope"] == "task_artifact" + assert response.json()["item_id"] == "artifact-1" + assert response.json()["item_context"] == { + "anchor": {"kind": "document"}, + "taskId": str(task.id), + "is_emoji": True, + } + + @mock.patch("posthog.api.comments.send_mention_notifications") + def test_personal_channel_comments_ignore_mentions(self, send_notifications: mock.Mock) -> None: + task = self._task_artifact_target() + task.channel.channel_type = "personal" + task.channel.save(update_fields=["channel_type"]) + mentioned = User.objects.create_and_join(self.organization, "private-mentioned@posthog.com", "password") + + response = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "This stays private @[Mentioned](private-mentioned@posthog.com)", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + "mentions": [mentioned.id], + }, + ) + + assert response.status_code == status.HTTP_201_CREATED + send_notifications.assert_not_called() + task_activity_model = apps.get_model("tasks", "TaskCommentActivity") + assert not task_activity_model.objects.unscoped().filter(team=self.team, user=mentioned, task=task).exists() + + @mock.patch("posthog.api.comments._record_task_comment_activity") + def test_edit_mentions_do_not_repeat_relationship_notifications(self, record_activity: mock.Mock) -> None: + task = self._task_artifact_target() + mentioned = User.objects.create_and_join(self.organization, "mentioned@posthog.com", "password") + created = self.client.post( + f"/api/projects/{self.team.id}/comments", + { + "content": "Old comment", + "scope": "task_artifact", + "item_id": "artifact-1", + "item_context": {"anchor": {"kind": "document"}, "taskId": str(task.id)}, + }, + ) + assert created.status_code == status.HTTP_201_CREATED + record_activity.reset_mock() + + response = self.client.patch( + f"/api/projects/{self.team.id}/comments/{created.json()['id']}" + f"?scope=task_artifact&item_id=artifact-1&task_id={task.id}", + {"content": "Edited mention", "mentions": [mentioned.id]}, + ) + + assert response.status_code == status.HTTP_200_OK + assert record_activity.call_args.kwargs["include_relationship_recipients"] is False + assert record_activity.call_args.kwargs["activity_at"] is not None + def _create_comment(self, data: dict | None = None) -> Any: if data is None: data = {} diff --git a/posthog/migrations/1287_oauthaccesstoken_sandbox_task_id.py b/posthog/migrations/1287_oauthaccesstoken_sandbox_task_id.py new file mode 100644 index 000000000000..4c272499d3b1 --- /dev/null +++ b/posthog/migrations/1287_oauthaccesstoken_sandbox_task_id.py @@ -0,0 +1,13 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("posthog", "1286_cleanup_orphaned_identity_provider_configs")] + + operations = [ + migrations.AddField( + model_name="oauthaccesstoken", + name="sandbox_task_id", + field=models.UUIDField(blank=True, null=True), + ), + ] diff --git a/posthog/migrations/max_migration.txt b/posthog/migrations/max_migration.txt index 0d28bac85434..4c32290de29f 100644 --- a/posthog/migrations/max_migration.txt +++ b/posthog/migrations/max_migration.txt @@ -1 +1 @@ -1286_cleanup_orphaned_identity_provider_configs +1287_oauthaccesstoken_sandbox_task_id diff --git a/posthog/models/oauth.py b/posthog/models/oauth.py index 696384830fe9..809b547a0510 100644 --- a/posthog/models/oauth.py +++ b/posthog/models/oauth.py @@ -523,6 +523,8 @@ class Meta(AbstractAccessToken.Meta): scoped_teams: ArrayField = ArrayField(models.IntegerField(), null=True, blank=True) scoped_organizations: ArrayField = ArrayField(models.CharField(max_length=100), null=True, blank=True) + # Server-minted sandbox binding: task-scoped APIs must not trust a caller-supplied task header alone. + sandbox_task_id: models.UUIDField = models.UUIDField(null=True, blank=True) # When set, this token was minted by a staff user impersonating `user`. Used to revoke # tokens at impersonation end. SET_NULL so the customer's tokens survive admin deactivation. diff --git a/posthog/temporal/oauth.py b/posthog/temporal/oauth.py index a3dc8fb7daa6..abbd863c54e8 100644 --- a/posthog/temporal/oauth.py +++ b/posthog/temporal/oauth.py @@ -1,5 +1,6 @@ from datetime import timedelta from typing import Literal +from uuid import UUID from django.conf import settings from django.utils import timezone @@ -41,11 +42,11 @@ # OAuth applications used to mint sandbox agent tokens. The Array applications also # issue interactive Desktop grants, so membership in this set does not prove sandbox origin. +POSTHOG_CODE_OAUTH_APP_CLIENT_IDS = frozenset({ARRAY_APP_CLIENT_ID_US, ARRAY_APP_CLIENT_ID_EU, ARRAY_APP_CLIENT_ID_DEV}) + SANDBOX_OAUTH_APP_CLIENT_IDS = frozenset( { - ARRAY_APP_CLIENT_ID_US, - ARRAY_APP_CLIENT_ID_EU, - ARRAY_APP_CLIENT_ID_DEV, + *POSTHOG_CODE_OAUTH_APP_CLIENT_IDS, POSTHOG_AI_APP_CLIENT_ID_US, POSTHOG_AI_APP_CLIENT_ID_EU, POSTHOG_AI_APP_CLIENT_ID_DEV, @@ -226,7 +227,9 @@ def get_sandbox_oauth_app(application: SandboxOAuthApplication = "array") -> OAu return get_array_app() -def _mint_oauth_access_token(user, team_id: int, *, app: OAuthApplication, scopes: list[str]) -> str: +def _mint_oauth_access_token( + user, team_id: int, *, app: OAuthApplication, scopes: list[str], sandbox_task_id: UUID | None = None +) -> str: token_value = generate_random_oauth_access_token(None) OAuthAccessToken.objects.create( @@ -236,6 +239,7 @@ def _mint_oauth_access_token(user, team_id: int, *, app: OAuthApplication, scope expires=timezone.now() + timedelta(seconds=TOKEN_EXPIRATION_SECONDS), scope=" ".join(dict.fromkeys(scopes)), scoped_teams=[team_id], + sandbox_task_id=sandbox_task_id, ) return token_value @@ -249,6 +253,7 @@ def create_oauth_access_token_for_user( include_internal_scopes: bool = True, include_mcp_builtin_agent_scope: bool = False, application: SandboxOAuthApplication = "array", + sandbox_task_id: UUID | None = None, ) -> str: resolved = resolve_scopes(scopes, include_internal_scopes=include_internal_scopes) if include_mcp_builtin_agent_scope: @@ -257,7 +262,7 @@ def create_oauth_access_token_for_user( # does not narrow the token's other scopes. resolved.append(MCP_BUILT_IN_AGENT_SCOPE) app = get_sandbox_oauth_app(application) - return _mint_oauth_access_token(user, team_id, app=app, scopes=list(resolved)) + return _mint_oauth_access_token(user, team_id, app=app, scopes=list(resolved), sandbox_task_id=sandbox_task_id) def get_wizard_app() -> OAuthApplication: diff --git a/posthog/temporal/tests/test_oauth.py b/posthog/temporal/tests/test_oauth.py index af7b552da7d9..a78da73e554f 100644 --- a/posthog/temporal/tests/test_oauth.py +++ b/posthog/temporal/tests/test_oauth.py @@ -1,3 +1,5 @@ +from uuid import uuid4 + from django.test import SimpleTestCase, TestCase, override_settings from parameterized import parameterized @@ -164,6 +166,18 @@ def test_posthog_ai_application_uses_dev_app(self) -> None: assert access_token.application_id == app.id assert access_token.scoped_teams == [team.id] + @override_settings(CLOUD_DEPLOYMENT="DEV") + def test_task_binding_is_persisted_only_when_supplied(self) -> None: + self._create_oauth_app(ARRAY_APP_CLIENT_ID_DEV, "Array Dev App") + user, team = self._create_user_and_team() + task_id = uuid4() + + bound = create_oauth_access_token_for_user(user, team.id, sandbox_task_id=task_id) + unbound = create_oauth_access_token_for_user(user, team.id) + + assert OAuthAccessToken.objects.get(token=bound).sandbox_task_id == task_id + assert OAuthAccessToken.objects.get(token=unbound).sandbox_task_id is None + @override_settings(CLOUD_DEPLOYMENT="DEV") def test_posthog_ai_application_requires_existing_app(self) -> None: user, team = self._create_user_and_team() diff --git a/products/canvas/backend/comment_access.py b/products/canvas/backend/comment_access.py new file mode 100644 index 000000000000..1a123cad45da --- /dev/null +++ b/products/canvas/backend/comment_access.py @@ -0,0 +1,30 @@ +from uuid import UUID + +from django.core.exceptions import ValidationError +from django.db.models import Q + +from products.canvas.backend.models import Canvas + + +def canvas_belongs_to_task(*, team_id: int, canvas_id: str, task_id: UUID) -> bool: + try: + return ( + Canvas.objects.for_team(team_id) + .filter(id=canvas_id, deleted=False) + .filter(Q(generation_task_id=task_id) | Q(source_versions__task_id=task_id)) + .exists() + ) + except (ValueError, ValidationError): + return False + + +def canvas_owner_id(*, team_id: int, canvas_id: str) -> int | None: + try: + return ( + Canvas.objects.for_team(team_id) + .filter(id=canvas_id, deleted=False) + .values_list("created_by_id", flat=True) + .first() + ) + except (ValueError, ValidationError): + return None diff --git a/products/canvas/backend/tests/test_cloud_builder.py b/products/canvas/backend/tests/test_cloud_builder.py index 2337bc96dc5f..a1bb2c00adb9 100644 --- a/products/canvas/backend/tests/test_cloud_builder.py +++ b/products/canvas/backend/tests/test_cloud_builder.py @@ -95,6 +95,15 @@ def test_runtime_uses_the_document_bound_message_port(self) -> None: self.assertIn('event.data?.type!=="connect"', runtime) self.assertIn("event.ports[0]", runtime) self.assertIn("port?.postMessage", runtime) + self.assertIn('event.data?.type==="set-comment-highlights"', runtime) + self.assertIn('CSS.highlights.set("posthog-canvas-comment"', runtime) + self.assertNotIn("ph-canvas-comment-outline", runtime) + self.assertIn('type:"comment-activate"', runtime) + self.assertIn("event.preventDefault();event.stopPropagation()", runtime) + self.assertIn("if(!items.length||timer)return", runtime) + self.assertNotIn("clearTimeout(timer);timer=setTimeout(()=>render(items),100)", runtime) + self.assertIn('document.addEventListener("selectionchange"', runtime) + self.assertNotIn('document.addEventListener("mouseup"', runtime) self.assertNotIn("parent.postMessage({channel,...message}", runtime) def test_runtime_bounds_host_side_effects(self) -> None: diff --git a/products/canvas/packages/canvas_builder/build.mjs b/products/canvas/packages/canvas_builder/build.mjs index 04f06d4e630e..c75adbc483b3 100644 --- a/products/canvas/packages/canvas_builder/build.mjs +++ b/products/canvas/packages/canvas_builder/build.mjs @@ -24,6 +24,8 @@ const forbiddenHtml = /(?:src|href)\s*=\s*["']\s*(javascript|data:text\/html|vbs const extensions = ['', '.ts', '.tsx', '.js', '.jsx', '.css', '.json', '.svg', '.txt'] const runtimePath = 'assets/canvas-runtime.js' const runtime = `(()=>{const channel="posthog-canvas",pending=new Map;let sequence=0,port;const post=(message)=>port?.postMessage({channel,...message});const call=(method,payload)=>new Promise((resolve,reject)=>{const id=String(++sequence);const timer=setTimeout(()=>{pending.delete(id);reject(new Error("Canvas request timed out"));},30000);pending.set(id,{resolve,reject,timer});post({type:"data-request",id,method,payload});});const receive=(event)=>{if(event.data?.channel!==channel||event.data?.type!=="data-response")return;const request=pending.get(event.data.id);if(!request)return;pending.delete(event.data.id);clearTimeout(request.timer);event.data.ok?request.resolve(event.data.result):request.reject(new Error(event.data.error??"Canvas request failed"));};const capture=(event,properties,distinctId)=>{const normalized=properties??{};let serialized;try{serialized=JSON.stringify(normalized)}catch{throw new Error("Canvas capture properties must be serializable")};if(typeof serialized!=="string"||serialized.length>16384)throw new Error("Canvas capture properties are too large");return call("capture",{event,properties:normalized,distinctId})};const openExternal=(value)=>{const url=new URL(value);if(url.protocol!=="https:"||!(url.hostname==="posthog.com"||url.hostname.endsWith(".posthog.com")))throw new Error("Canvas external URL is not allowed");post({type:"open-external",url:url.href})};window.ph={loadInsight:(shortId,options)=>call("loadInsight",{shortId,dateRange:options?.dateRange}),query:(query,params)=>call("query",typeof query==="string"?{hogql:query,params:params??{}}:{query,params:params??{}}),capture,openExternal};addEventListener("message",(event)=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",receive);port.start();if(document.readyState!=="loading")post({type:"ready"});if(document.readyState==="complete")post({type:"rendered"});});addEventListener("error",(event)=>post({type:"error",message:event.message||"Canvas runtime error",stack:event.error?.stack}));addEventListener("unhandledrejection",(event)=>post({type:"error",message:event.reason instanceof Error?event.reason.message:String(event.reason),stack:event.reason instanceof Error?event.reason.stack:undefined}));addEventListener("DOMContentLoaded",()=>post({type:"ready"}));addEventListener("load",()=>post({type:"rendered"}));})();` +const selectionRuntime = `(()=>{const channel="posthog-canvas";let port,timer=0;const post=message=>port?.postMessage({channel,...message}),clear=()=>post({type:"text-selection-cleared"}),report=()=>{clearTimeout(timer);timer=setTimeout(()=>{const selection=getSelection();if(!selection||selection.isCollapsed||selection.rangeCount===0){clear();return}const range=selection.getRangeAt(0);if(!document.body.contains(range.startContainer)||!document.body.contains(range.endContainer)){clear();return}const before=document.createRange();before.selectNodeContents(document.body);before.setEnd(range.startContainer,range.startOffset);const through=document.createRange();through.selectNodeContents(document.body);through.setEnd(range.endContainer,range.endOffset);const whole=document.createRange();whole.selectNodeContents(document.body);const text=whole.toString(),start=before.toString().length,end=through.toString().length,quote=text.slice(start,end);if(!quote.trim()||quote.length>10000){clear();return}const rect=range.getBoundingClientRect();post({type:"text-selection",selection:{quote,prefix:text.slice(Math.max(0,start-32),start),suffix:text.slice(end,end+32),start,end,rect:{top:rect.top,right:rect.right,bottom:rect.bottom,left:rect.left}}})},80)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0]});document.addEventListener("selectionchange",report)})();` +const highlightRuntime = `(()=>{const channel="posthog-canvas",style=document.createElement("style");style.textContent="::highlight(posthog-canvas-comment){background:rgba(250,204,21,.32);color:inherit}::highlight(posthog-canvas-comment-active){background:rgba(250,204,21,.48);color:inherit}";document.head.appendChild(style);let items=[],ranges=[],port,timer=0;const indexText=()=>{const walker=document.createTreeWalker(document.body,NodeFilter.SHOW_TEXT),entries=[];let text="";for(let node=walker.nextNode();node;node=walker.nextNode()){const start=text.length;text+=node.data;entries.push({node,start,end:text.length})}return{text,entries}},rangeAt=(index,start,end)=>{const find=offset=>{let low=0,high=index.entries.length-1,match=null;while(low<=high){const middle=low+high>>1,entry=index.entries[middle];if(offsetentry.end)low=middle+1;else{match=entry;high=middle-1}}return match},startEntry=find(start),endEntry=find(end);if(!startEntry||!endEntry)return null;const range=document.createRange();range.setStart(startEntry.node,start-startEntry.start);range.setEnd(endEntry.node,end-endEntry.start);return range},resolve=(text,anchor)=>{if(text.slice(anchor.start,anchor.end)===anchor.quote)return{start:anchor.start,end:anchor.end};const matches=[];for(let start=text.indexOf(anchor.quote);start>=0;start=text.indexOf(anchor.quote,start+Math.max(anchor.quote.length,1))){const end=start+anchor.quote.length,prefix=text.slice(Math.max(0,start-anchor.prefix.length),start),suffix=text.slice(end,end+anchor.suffix.length);matches.push({start,end,score:(anchor.prefix&&prefix===anchor.prefix?2:0)+(anchor.suffix&&suffix===anchor.suffix?2:0)})}if(matches.length===1)return matches[0];matches.sort((a,b)=>b.score-a.score);return matches[0]?.score&&matches[0].score!==matches[1]?.score?matches[0]:null},render=next=>{items=next||[];ranges=[];if(!window.Highlight||!window.CSS||!CSS.highlights)return;const normal=new Highlight,active=new Highlight,index=indexText();for(const item of items){const hit=resolve(index.text,item.anchor),range=hit&&rangeAt(index,hit.start,hit.end);if(range){ranges.push({id:item.id,range});(item.active?active:normal).add(range)}}CSS.highlights.set("posthog-canvas-comment",normal);CSS.highlights.set("posthog-canvas-comment-active",active)};addEventListener("message",event=>{if(port||event.source!==parent||event.data?.channel!==channel||event.data?.type!=="connect"||!event.ports[0])return;port=event.ports[0];port.addEventListener("message",event=>{if(event.data?.channel===channel&&event.data?.type==="set-comment-highlights")render(event.data.highlights)});port.start()});document.addEventListener("click",event=>{for(const item of ranges)for(const rect of item.range.getClientRects())if(event.clientX>=rect.left&&event.clientX<=rect.right&&event.clientY>=rect.top&&event.clientY<=rect.bottom){event.preventDefault();event.stopPropagation();port?.postMessage({channel,type:"comment-activate",id:item.id});return}},true);new MutationObserver(()=>{if(!items.length||timer)return;timer=setTimeout(()=>{timer=0;render(items)},100)}).observe(document.body,{childList:true,characterData:true,subtree:true})})();` const platformStylesheet = ` @import "tailwindcss"; @import "@posthog/quill/tokens.css"; @@ -338,7 +340,7 @@ async function buildCanvas(project) { } const cssPath = `assets/canvas-platform-${sha256(platformCss).slice(0, 10)}.css` files.push(artifact(cssPath, platformCss)) - files.push(artifact(runtimePath, runtime)) + files.push(artifact(runtimePath, `${runtime}\n${selectionRuntime}\n${highlightRuntime}`)) const head = `` html = html.includes('') ? html.replace('', `${head}`) : `${head}${html}` files.unshift(artifact(project.entryHtml, html)) diff --git a/products/desktop/packages/agent/src/server/agent-server.test.ts b/products/desktop/packages/agent/src/server/agent-server.test.ts index c73cc5a62a40..fc67ed82c06c 100644 --- a/products/desktop/packages/agent/src/server/agent-server.test.ts +++ b/products/desktop/packages/agent/src/server/agent-server.test.ts @@ -3988,6 +3988,7 @@ describe("AgentServer HTTP Mode", () => { expect(prompt).not.toContain("Create a draft pull request"); expect(prompt).toContain("Generated-By: PostHog Code"); expect(prompt).toContain("Task-Id: test-task-id"); + expect(prompt).toContain("Follow `next` until it is null"); }); it("returns default prompt when no prUrl", () => { @@ -3999,6 +4000,7 @@ describe("AgentServer HTTP Mode", () => { ); expect(prompt).toContain("Generated-By: PostHog Code"); expect(prompt).toContain("Task-Id: test-task-id"); + expect(prompt).toContain("Follow `next` until it is null"); expect(prompt).not.toContain("gh pr create --draft"); // If the user does explicitly ask for a PR in this review-first mode, // the agent must still use the PostHog Code footer, not Claude Code's default. @@ -4037,6 +4039,11 @@ describe("AgentServer HTTP Mode", () => { "Closes #", "Generated-By: PostHog Code", "Task-Id: test-task-id", + "tasks-artifacts-list", + "tasks-comments-list", + "tasks-comments-retrieve", + "Follow `next` until it is null", + "filter comments by artifact", ], shouldNotContain: [], }, @@ -4064,6 +4071,7 @@ describe("AgentServer HTTP Mode", () => { for (const text of shouldNotContain) { expect(prompt).not.toContain(text); } + expect(prompt).toContain("Follow `next` until it is null"); }, ); @@ -4076,6 +4084,7 @@ describe("AgentServer HTTP Mode", () => { expect(prompt).toContain("gh pr create --draft"); expect(prompt).toContain("Generated-By: PostHog Code"); expect(prompt).toContain("Task-Id: test-task-id"); + expect(prompt).toContain("Follow `next` until it is null"); // Slack-origin PRs are attributed to PostHog, not the PostHog Code app. expect(prompt).toContain( "Created with [PostHog](https://posthog.com?ref=pr)", diff --git a/products/desktop/packages/agent/src/server/agent-server.ts b/products/desktop/packages/agent/src/server/agent-server.ts index 53d5fd8527da..2c9624e79207 100644 --- a/products/desktop/packages/agent/src/server/agent-server.ts +++ b/products/desktop/packages/agent/src/server/agent-server.ts @@ -3573,6 +3573,10 @@ To ping a Slack user, reuse a \`<@U…|displayname>\` token that already appears You can also open pull requests directly from this Slack thread. When the user's question describes a problem with a plausible code-side fix — a bug visible in errors or logs, missing or broken instrumentation, a broken funnel step traceable to UI code, a stale config that lives in a repo — end your reply with a one-sentence offer to open a PR for the fix and ask if they want you to proceed. Skip the offer for pure data lookups with no actionable code change (e.g. "what was DAU yesterday?"), and skip it when the fix would clearly live outside any repo you can reach. ` : ""; + const currentTaskCommentsInstructions = ` +## Current task comments +When the \`tasks-comments-*\` tools are available, they read human comments only from this task and its artifacts or canvases. Call \`tasks-comments-list\` directly when the user asks you to review comments or says they added feedback. Follow \`next\` until it is null when reviewing all comments or replies. Use \`tasks-artifacts-list\` when you need the artifact inventory or want to filter comments by artifact, and call \`tasks-comments-retrieve\` to read every reply before acting on a root comment.`; + const signedCommitInstructions = ` ## Committing (signed commits required) Commits MUST be signed. \`git commit\` and \`git push\` are blocked in this environment. @@ -3672,7 +3676,7 @@ Do the requested work, but stop with local changes ready for review. Important: - Do NOT create new commits, push to the branch, or update the pull request unless the user explicitly asks. - Do NOT create a new branch or a new pull request unless the user explicitly asks. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} +${currentTaskCommentsInstructions}${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3693,7 +3697,7 @@ After completing the requested changes: Important: - Do NOT create a new branch or a new pull request unless the user explicitly asks. - Do NOT push fixes for review comments without replying to and resolving each related thread. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} +${currentTaskCommentsInstructions}${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3745,7 +3749,7 @@ ${publishInstructions} Important: - Prefer using MCP tools to answer questions with real data over giving generic advice. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} +${currentTaskCommentsInstructions}${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3765,7 +3769,7 @@ ${publicRepoSafetyInstruction.trimStart()} ${prMentionSafetyInstruction.trimStart()} - End the PR description with a horizontal rule followed by this footer line: ${prFooter} - Always create the PR as a draft. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} +${currentTaskCommentsInstructions}${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3795,7 +3799,7 @@ ${prFooter} Important: - Always create the PR as a draft. Do not ask for confirmation. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} +${currentTaskCommentsInstructions}${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } diff --git a/products/desktop/packages/api-client/src/generated.ts b/products/desktop/packages/api-client/src/generated.ts index fc57711b82c0..05490a7e78fb 100644 --- a/products/desktop/packages/api-client/src/generated.ts +++ b/products/desktop/packages/api-client/src/generated.ts @@ -4366,7 +4366,7 @@ export namespace Schemas { export type ColorMode = "light" | "dark"; export type Comment = { id: string; - created_by: UserBasic & unknown; + created_by: (UserBasic & unknown) | null; deleted?: (boolean | null) | undefined; mentions?: Array | undefined; slug?: string | undefined; @@ -4375,9 +4375,10 @@ export namespace Schemas { version: number; created_at: string; item_id?: (string | null) | undefined; - item_context?: null | undefined; + item_context?: unknown; scope: string; source_comment?: (string | null) | undefined; + completed_at?: (string | null) | undefined; }; export type CompareItem = { label: string; value: string }; export type ConclusionEnum = "won" | "lost" | "inconclusive" | "stopped_early" | "invalid"; @@ -18454,7 +18455,7 @@ export namespace Endpoints { path: "/api/projects/{project_id}/comments/"; requestFormat: "json"; parameters: { - query: Partial<{ cursor: string; item_id: string; scope: string; search: string; source_comment: string }>; + query: Partial<{ cursor: string; item_id: string; task_id: string; scope: string; search: string; source_comment: string }>; path: { project_id: string }; }; responses: { 200: Schemas.PaginatedCommentList }; diff --git a/products/desktop/packages/api-client/src/posthog-client.ts b/products/desktop/packages/api-client/src/posthog-client.ts index e427e84349a5..725c0b0eb222 100644 --- a/products/desktop/packages/api-client/src/posthog-client.ts +++ b/products/desktop/packages/api-client/src/posthog-client.ts @@ -199,6 +199,29 @@ export interface TaskSessionStorageAccess { content_sha256: string | null; } +/** + * The commentable resources this client knows how to address. `scope` is a + * free-form column on the backend `Comment` model, so adding a resource is a + * new member here plus a caller — no migration and no endpoint. + */ +export type CommentScope = "task_artifact" | "desktop_canvas" | "task"; + +/** Named `Resource*` so it never collides with the DOM's global `Comment`. + * Optimistic rows do not have a server version yet, while item_context is a + * real JSON value despite the generated serializer's historically narrow type. */ +export type ResourceComment = Omit & { + version?: number; +}; + +export interface CreateResourceCommentRequest { + scope: CommentScope; + itemId: string; + content: string; + context: unknown; + sourceCommentId?: string; + mentions?: number[]; +} + /** Thrown when the backend rejects a cloud run with a 429 usage-limit error. */ export class CloudUsageLimitError extends Error { limitType: UsageLimitType; @@ -2717,8 +2740,7 @@ export class PostHogAPIClient { return (await response.json()) as TaskMention[]; } - // Tasks the current user is involved in (created, mentioned, or messaged), - // one row per task, newest activity first. + // Task lifecycle and individual comment activity, newest first. async getTaskActivity(options?: { before?: string; beforeId?: string; @@ -2741,8 +2763,7 @@ export class PostHogAPIClient { return (await response.json()) as TaskActivityPage; } - // Read state is per task, so callers name the tasks the user has seen rather than - // clearing the whole feed. + // Task lifecycle activity clears by task timestamp; comment activity clears by row id. async markTaskActivityRead( activities: TaskActivityReadMarker[], ): Promise { @@ -3236,6 +3257,48 @@ export class PostHogAPIClient { return data.url; } + async getResourceComments( + scope: CommentScope, + itemId: string, + taskId: string, + ): Promise { + const teamId = await this.getTeamId(); + const comments: ResourceComment[] = []; + let cursor: string | undefined; + do { + const page = await this.api.get("/api/projects/{project_id}/comments/", { + path: { project_id: String(teamId) }, + query: { scope, item_id: itemId, task_id: taskId, cursor }, + }); + comments.push(...page.results); + cursor = page.next + ? (new URL(page.next).searchParams.get("cursor") ?? undefined) + : undefined; + } while (cursor); + return comments; + } + + async createResourceComment( + request: CreateResourceCommentRequest, + ): Promise { + const teamId = await this.getTeamId(); + const payload = { + content: request.content, + scope: request.scope, + item_id: request.itemId, + item_context: request.context, + source_comment: request.sourceCommentId ?? null, + mentions: request.mentions ?? [], + // Resolution is represented by a thread-state reply so this stays on the + // same PAT-compatible write path as ordinary comments. + is_task: false, + }; + return await this.api.post("/api/projects/{project_id}/comments/", { + path: { project_id: String(teamId) }, + body: payload as unknown as Schemas.Comment, + }); + } + async getTaskSessionStorageAccess( taskId: string, runId: string, diff --git a/products/desktop/packages/core/src/canvas/freeformSchemas.test.ts b/products/desktop/packages/core/src/canvas/freeformSchemas.test.ts index 7f1a8381e79c..2c3cf384117e 100644 --- a/products/desktop/packages/core/src/canvas/freeformSchemas.test.ts +++ b/products/desktop/packages/core/src/canvas/freeformSchemas.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from "vitest"; -import { canvasToHostMessageSchema } from "./freeformSchemas"; +import { + canvasToHostMessageSchema, + hostToCanvasMessageSchema, +} from "./freeformSchemas"; -describe("canvasToHostMessageSchema open-external", () => { +describe("canvasToHostMessageSchema", () => { const message = (url: string) => ({ channel: "posthog-canvas", type: "open-external", @@ -32,4 +35,68 @@ describe("canvasToHostMessageSchema open-external", () => { false, ); }); + + it("accepts a bounded text selection and rejects oversized selected text", () => { + const selection = { + channel: "posthog-canvas", + type: "text-selection", + selection: { + quote: "selected text", + prefix: "before ", + suffix: " after", + start: 7, + end: 20, + rect: { top: 10, right: 80, bottom: 30, left: 20 }, + }, + }; + + expect(canvasToHostMessageSchema.safeParse(selection).success).toBe(true); + expect( + canvasToHostMessageSchema.safeParse({ + ...selection, + selection: { ...selection.selection, quote: "x".repeat(10_001) }, + }).success, + ).toBe(false); + }); + + it("accepts an explicit selection-cleared event", () => { + expect( + canvasToHostMessageSchema.safeParse({ + channel: "posthog-canvas", + type: "text-selection-cleared", + }).success, + ).toBe(true); + }); + + it("accepts a bounded comment activation", () => { + expect( + canvasToHostMessageSchema.safeParse({ + channel: "posthog-canvas", + type: "comment-activate", + id: "comment-1", + }).success, + ).toBe(true); + }); + + it("accepts bounded comment highlights", () => { + expect( + hostToCanvasMessageSchema.safeParse({ + channel: "posthog-canvas", + type: "set-comment-highlights", + highlights: [ + { + id: "comment-1", + active: false, + anchor: { + quote: "selected text", + prefix: "before ", + suffix: " after", + start: 7, + end: 20, + }, + }, + ], + }).success, + ).toBe(true); + }); }); diff --git a/products/desktop/packages/core/src/canvas/freeformSchemas.ts b/products/desktop/packages/core/src/canvas/freeformSchemas.ts index b5ee108a51a8..396a260d29fa 100644 --- a/products/desktop/packages/core/src/canvas/freeformSchemas.ts +++ b/products/desktop/packages/core/src/canvas/freeformSchemas.ts @@ -1,5 +1,6 @@ import { isSafePostHogUrl } from "@posthog/shared"; import { z } from "zod"; +import { textCommentAnchorDataSchema } from "../comments/anchors"; // The template id for freeform-React canvases. Stored on a canvas's meta so the // generation path can resolve the right system prompt. @@ -126,6 +127,28 @@ export type CanvasAnalyticsConfig = z.infer; export const canvasThemeSchema = z.enum(["light", "dark"]); export type CanvasTheme = z.infer; +const canvasTextSelectionDataSchema = textCommentAnchorDataSchema.extend({ + rect: z.object({ + top: z.number().finite(), + right: z.number().finite(), + bottom: z.number().finite(), + left: z.number().finite(), + }), +}); +export const canvasTextSelectionSchema = canvasTextSelectionDataSchema.refine( + ({ start, end }) => end > start, +); +export type CanvasTextSelection = z.infer; + +export const canvasCommentHighlightSchema = z.object({ + id: z.string().min(1).max(128), + active: z.boolean(), + anchor: textCommentAnchorDataSchema.refine(({ start, end }) => end > start), +}); +export type CanvasCommentHighlight = z.infer< + typeof canvasCommentHighlightSchema +>; + // host -> iframe export const hostToCanvasMessageSchema = z.discriminatedUnion("type", [ // First frame: hand the iframe its source + the run mode. The iframe does not @@ -143,6 +166,7 @@ export const hostToCanvasMessageSchema = z.discriminatedUnion("type", [ // already correct; live theme changes use the `set-theme` frame below // (which re-themes without remounting). Absent = light. theme: canvasThemeSchema.optional(), + highlights: z.array(canvasCommentHighlightSchema).max(500).optional(), }), // Live theme change: re-apply light/dark WITHOUT remounting the app. Sent on // its own (not folded into `init`) so toggling the host theme — or an OS @@ -152,6 +176,11 @@ export const hostToCanvasMessageSchema = z.discriminatedUnion("type", [ type: z.literal("set-theme"), theme: canvasThemeSchema, }), + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("set-comment-highlights"), + highlights: z.array(canvasCommentHighlightSchema).max(500), + }), // Reply to a data-request, correlated by `id`. z.object({ channel: z.literal(CANVAS_CHANNEL), @@ -223,5 +252,19 @@ export const canvasToHostMessageSchema = z.discriminatedUnion("type", [ type: z.literal("open-external"), url: z.string().refine(isSafePostHogUrl), }), + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("text-selection"), + selection: canvasTextSelectionSchema, + }), + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("text-selection-cleared"), + }), + z.object({ + channel: z.literal(CANVAS_CHANNEL), + type: z.literal("comment-activate"), + id: z.string().min(1).max(128), + }), ]); export type CanvasToHostMessage = z.infer; diff --git a/products/desktop/packages/core/src/canvas/taskActivity.test.ts b/products/desktop/packages/core/src/canvas/taskActivity.test.ts index 09a00c8d3ca0..ebd8f1aa8d4d 100644 --- a/products/desktop/packages/core/src/canvas/taskActivity.test.ts +++ b/products/desktop/packages/core/src/canvas/taskActivity.test.ts @@ -40,11 +40,30 @@ describe("toTaskActivityItems", () => { snippet: "ping @[Me](me@posthog.com)", author: ann, messageId: "m1", + commentId: null, + commentTarget: null, isUnread: true, }, ]); }); + it("maps a comment activity target for exact navigation", () => { + const [item] = toTaskActivityItems([ + activity({ + latest_message_id: null, + latest_comment_id: "comment-1", + latest_comment_scope: "task_artifact", + latest_comment_item_id: "artifact-1", + }), + ]); + + expect(item.commentId).toBe("comment-1"); + expect(item.commentTarget).toEqual({ + scope: "task_artifact", + itemId: "artifact-1", + }); + }); + it("labels untitled tasks and tolerates missing optional values", () => { const [item] = toTaskActivityItems([ activity({ diff --git a/products/desktop/packages/core/src/canvas/taskActivity.ts b/products/desktop/packages/core/src/canvas/taskActivity.ts index 0d4b9ab422e1..d251ef43708d 100644 --- a/products/desktop/packages/core/src/canvas/taskActivity.ts +++ b/products/desktop/packages/core/src/canvas/taskActivity.ts @@ -3,12 +3,13 @@ import type { TaskActivityKind, UserBasic, } from "@posthog/shared/domain-types"; +import type { CommentTarget } from "../comments/anchors"; /** * The Activity feed — tasks the current user is involved in (created, mentioned * in, or messaged in) — as served by the backend task-activity index - * (`getTaskActivity`). One row per task, newest activity first; the client only - * maps DTOs to items. + * (`getTaskActivity`). Task state collapses per task, while comment notifications + * are individual entries; the client only maps DTOs to items. */ export interface TaskActivityItem { @@ -25,6 +26,8 @@ export interface TaskActivityItem { snippet: string; author: UserBasic | null; messageId: string | null; + commentId?: string | null; + commentTarget?: CommentTarget | null; isUnread: boolean; } @@ -43,6 +46,14 @@ export function toTaskActivityItems( snippet: row.snippet, author: row.latest_author ?? null, messageId: row.latest_message_id ?? null, + commentId: row.latest_comment_id ?? null, + commentTarget: + row.latest_comment_scope && row.latest_comment_item_id + ? { + scope: row.latest_comment_scope as CommentTarget["scope"], + itemId: row.latest_comment_item_id, + } + : null, isUnread: row.is_unread, })); } diff --git a/products/desktop/packages/core/src/comments/anchors.test.ts b/products/desktop/packages/core/src/comments/anchors.test.ts new file mode 100644 index 000000000000..2da4660705ec --- /dev/null +++ b/products/desktop/packages/core/src/comments/anchors.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from "vitest"; +import { + createTextCommentAnchor, + isThreadResolved, + parseCommentContext, + resolveTextCommentAnchor, +} from "./anchors"; + +describe("artifact text anchors", () => { + it("creates and resolves a verified positional anchor", () => { + const text = "Before selected words after"; + const anchor = createTextCommentAnchor(text, 7, 21); + + if (!anchor) throw new Error("Expected an anchor"); + expect(resolveTextCommentAnchor(text, anchor)).toEqual({ + start: 7, + end: 21, + status: "exact", + }); + }); + + it("reanchors a quote after surrounding content changes", () => { + const original = "Before selected words after"; + const anchor = createTextCommentAnchor(original, 7, 21); + if (!anchor) throw new Error("Expected an anchor"); + const changed = `New introduction. ${original}`; + + expect(resolveTextCommentAnchor(changed, anchor)).toEqual({ + start: 25, + end: 39, + status: "reanchored", + }); + }); + + it("uses context to disambiguate repeated quotes", () => { + const original = "first repeated phrase then second repeated phrase end"; + const start = original.lastIndexOf("repeated phrase"); + const anchor = createTextCommentAnchor( + original, + start, + start + "repeated phrase".length, + ); + if (!anchor) throw new Error("Expected an anchor"); + const changed = `prefix ${original}`; + + expect(resolveTextCommentAnchor(changed, anchor)?.start).toBe( + changed.lastIndexOf("repeated phrase"), + ); + }); + + it("orphans deleted and ambiguous text instead of guessing", () => { + const deleted = createTextCommentAnchor("unique text", 0, 6); + if (!deleted) throw new Error("Expected an anchor"); + expect(resolveTextCommentAnchor("replacement", deleted)).toBeNull(); + + const ambiguous = { + kind: "text" as const, + quote: "same", + prefix: "", + suffix: "", + start: 100, + end: 104, + }; + expect(resolveTextCommentAnchor("same x same", ambiguous)).toBeNull(); + }); + + it("rejects whitespace-only selections", () => { + expect(createTextCommentAnchor("a b", 1, 4)).toBeNull(); + }); + + it("rejects selections larger than the persisted anchor contract", () => { + const text = "x".repeat(10_001); + expect(createTextCommentAnchor(text, 0, text.length)).toBeNull(); + }); + + it("validates versioned comment context and anchor bounds", () => { + expect( + parseCommentContext({ + anchor: { kind: "document" }, + canvasVersionId: "version-2", + }), + ).toEqual({ + anchor: { kind: "document" }, + canvasVersionId: "version-2", + }); + expect( + parseCommentContext({ + anchor: { + kind: "text", + quote: "x".repeat(10_001), + prefix: "", + suffix: "", + start: 0, + end: 10_001, + }, + }), + ).toBeNull(); + }); + + it("uses the latest thread-state event for resolution", () => { + const root = { completed_at: null }; + const event = (state: "resolved" | "open", created_at: string) => ({ + created_at, + item_context: { + anchor: { kind: "document" as const }, + threadState: state, + }, + }); + + expect( + isThreadResolved(root, [ + event("resolved", "2026-01-01T00:00:00Z"), + event("open", "2026-01-01T00:01:00Z"), + ]), + ).toBe(false); + expect( + isThreadResolved(root, [ + event("open", "2026-01-01T00:00:00Z"), + event("resolved", "2026-01-01T00:01:00Z"), + ]), + ).toBe(true); + }); +}); diff --git a/products/desktop/packages/core/src/comments/anchors.ts b/products/desktop/packages/core/src/comments/anchors.ts new file mode 100644 index 000000000000..ff3458986a00 --- /dev/null +++ b/products/desktop/packages/core/src/comments/anchors.ts @@ -0,0 +1,189 @@ +import type { CommentScope } from "@posthog/api-client/posthog-client"; +import { z } from "zod"; + +const CONTEXT_LENGTH = 32; +const MAX_QUOTE_LENGTH = 10_000; + +/** + * Addresses one commentable thing. `itemId` must be the resource's STABLE id + * (an artifact id, a canvas row id) — never a name or a version, so comments + * survive renames and reverts. + */ +export type CommentTarget = { + scope: CommentScope; + itemId: string; +}; + +/** The target as one string, for map keys and cache-key membership tests. */ +export function commentTargetKey(target: CommentTarget): string { + return `${target.scope}:${target.itemId}`; +} + +export function isSameCommentTarget( + a: CommentTarget | null, + b: CommentTarget | null, +): boolean { + return a?.scope === b?.scope && a?.itemId === b?.itemId; +} + +export const textCommentAnchorDataSchema = z.object({ + quote: z.string().min(1).max(MAX_QUOTE_LENGTH), + prefix: z.string().max(CONTEXT_LENGTH), + suffix: z.string().max(CONTEXT_LENGTH), + start: z.number().int().nonnegative(), + end: z.number().int().positive(), +}); + +export const textCommentAnchorSchema = textCommentAnchorDataSchema + .extend({ + kind: z.literal("text"), + }) + .refine(({ start, end }) => end > start, { + message: "Text anchor end must follow its start", + }); + +export const regionCommentAnchorSchema = z.object({ + kind: z.literal("region"), + x: z.number().min(0).max(1), + y: z.number().min(0).max(1), + width: z.number().min(0).max(1), + height: z.number().min(0).max(1), +}); + +const documentCommentAnchorSchema = z.object({ + kind: z.literal("document"), +}); + +export const commentAnchorSchema = z.discriminatedUnion("kind", [ + textCommentAnchorSchema, + regionCommentAnchorSchema, + documentCommentAnchorSchema, +]); + +export type TextCommentAnchor = z.infer; +export type RegionCommentAnchor = z.infer; +export type CommentAnchor = z.infer; + +export const commentContextSchema = z.object({ + anchor: commentAnchorSchema, + threadState: z.enum(["resolved", "open"]).optional(), + /** Immutable canvas source version rendered when the comment was made. */ + canvasVersionId: z.string().min(1).optional(), + // The task the commented resource belongs to. Artifact and canvas ids live in a run's + // JSON rather than a table, so the server can't get back to the task without being told. + taskId: z.string().optional(), +}); + +export type CommentContext = z.infer; + +export function parseCommentContext(value: unknown): CommentContext | null { + const parsed = commentContextSchema.safeParse(value); + return parsed.success ? parsed.data : null; +} + +export type ThreadStateComment = { + created_at: string; + item_context?: unknown; +}; + +export function isThreadResolved( + root: { completed_at?: string | null }, + replies: ThreadStateComment[], +): boolean { + const latestState = replies + .map((comment) => ({ + createdAt: comment.created_at, + state: parseCommentContext(comment.item_context)?.threadState, + })) + .filter( + ( + entry, + ): entry is { + createdAt: string; + state: "resolved" | "open"; + } => !!entry.state, + ) + .sort((a, b) => a.createdAt.localeCompare(b.createdAt)) + .at(-1)?.state; + return latestState ? latestState === "resolved" : !!root.completed_at; +} + +export type ResolvedTextAnchor = { + start: number; + end: number; + status: "exact" | "reanchored"; +}; + +export function createTextCommentAnchor( + text: string, + start: number, + end: number, +): TextCommentAnchor | null { + const safeStart = Math.max(0, Math.min(start, text.length)); + const safeEnd = Math.max(safeStart, Math.min(end, text.length)); + const quote = text.slice(safeStart, safeEnd); + if (!quote.trim() || quote.length > MAX_QUOTE_LENGTH) return null; + + return { + kind: "text", + quote, + prefix: text.slice(Math.max(0, safeStart - CONTEXT_LENGTH), safeStart), + suffix: text.slice(safeEnd, safeEnd + CONTEXT_LENGTH), + start: safeStart, + end: safeEnd, + }; +} + +/** + * Resolve a persisted text quote without ever guessing. The stored position is + * verified first. If content moved, prefix/suffix disambiguate quote matches; + * ties are deliberately treated as orphaned. + */ +export function resolveTextCommentAnchor( + text: string, + anchor: TextCommentAnchor, +): ResolvedTextAnchor | null { + if (text.slice(anchor.start, anchor.end) === anchor.quote) { + return { start: anchor.start, end: anchor.end, status: "exact" }; + } + + const candidates: number[] = []; + let from = 0; + while (from <= text.length - anchor.quote.length) { + const match = text.indexOf(anchor.quote, from); + if (match < 0) break; + candidates.push(match); + from = match + Math.max(anchor.quote.length, 1); + } + if (candidates.length === 0) return null; + if (candidates.length === 1) { + const start = candidates[0]; + return { + start, + end: start + anchor.quote.length, + status: "reanchored", + }; + } + + const ranked = candidates + .map((start) => { + const end = start + anchor.quote.length; + const prefix = text.slice( + Math.max(0, start - anchor.prefix.length), + start, + ); + const suffix = text.slice(end, end + anchor.suffix.length); + let score = 0; + if (anchor.prefix && prefix === anchor.prefix) score += 2; + if (anchor.suffix && suffix === anchor.suffix) score += 2; + return { start, score }; + }) + .sort((a, b) => b.score - a.score); + if (ranked[0].score === 0 || ranked[0].score === ranked[1].score) return null; + + return { + start: ranked[0].start, + end: ranked[0].start + anchor.quote.length, + status: "reanchored", + }; +} diff --git a/products/desktop/packages/core/src/panels/panelStoreHelpers.ts b/products/desktop/packages/core/src/panels/panelStoreHelpers.ts index 2b41453240bf..aaff4ad4b2ad 100644 --- a/products/desktop/packages/core/src/panels/panelStoreHelpers.ts +++ b/products/desktop/packages/core/src/panels/panelStoreHelpers.ts @@ -62,6 +62,35 @@ export function getLeafPanel( return panel?.type === "leaf" ? panel : null; } +/** + * The artifact the user is looking at, if any: the focused panel's active tab + * when that is an artifact, else any other panel's. Lets a pane elsewhere (the + * task's comment list) narrow itself to whatever is on screen. + */ +export function activeArtifactId(layout: TaskLayout): string | null { + const activeArtifact = (node: PanelNode): string | null => { + if (node.type !== "leaf") { + for (const child of node.children) { + const found = activeArtifact(child); + if (found) return found; + } + return null; + } + const active = node.content.tabs.find( + (tab) => tab.id === node.content.activeTabId, + ); + return active?.data.type === "artifact" ? active.data.artifactId : null; + }; + + const focused = layout.focusedPanelId + ? getLeafPanel(layout.panelTree, layout.focusedPanelId) + : null; + return ( + (focused ? activeArtifact(focused) : null) ?? + activeArtifact(layout.panelTree) + ); +} + export function getGroupPanel( tree: PanelNode, panelId: string, diff --git a/products/desktop/packages/core/src/sessions/sessionService.ts b/products/desktop/packages/core/src/sessions/sessionService.ts index cf59a5d3196e..858afba6f7a7 100644 --- a/products/desktop/packages/core/src/sessions/sessionService.ts +++ b/products/desktop/packages/core/src/sessions/sessionService.ts @@ -9,6 +9,10 @@ import type { SessionConfigSelectOption, SessionUpdate, } from "@agentclientprotocol/sdk"; +import type { + CreateResourceCommentRequest, + ResourceComment, +} from "@posthog/api-client/posthog-client"; import { type AcpMessage, type Adapter, @@ -49,6 +53,7 @@ import { isTerminalStatus, type Task, } from "@posthog/shared/domain-types"; +import type { CommentTarget } from "../comments/anchors"; import type { SpeechKind, SpeechSource } from "../speech/identifiers"; import { CONTEXT_WINDOW_OPTION_CATEGORY, @@ -7495,6 +7500,64 @@ export class SessionService { } } + async getResourceComments( + target: CommentTarget, + taskId: string, + ): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready") return []; + return authStatus.auth.client.getResourceComments( + target.scope, + target.itemId, + taskId, + ); + } + + /** + * Comments for several resources at once, for surfaces that centralize threads + * across a task's artifacts and canvases. Returns one flat list — every row + * already carries `scope` and `item_id`, so callers group without bookkeeping. + * Fanning out here (rather than in a hook) keeps the multi-source read in a + * service and lets the caller hold a single query. + */ + async getResourceCommentsForTargets( + targets: CommentTarget[], + taskId: string, + ): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready" || targets.length === 0) return []; + const client = authStatus.auth.client; + const pages: ResourceComment[][] = Array.from( + { length: targets.length }, + () => [], + ); + let nextIndex = 0; + const worker = async () => { + while (nextIndex < targets.length) { + const index = nextIndex++; + const target = targets[index]; + pages[index] = await client + .getResourceComments(target.scope, target.itemId, taskId) + // One unreadable resource must not blank the whole pane. + .catch(() => [] as ResourceComment[]); + } + }; + await Promise.all( + Array.from({ length: Math.min(4, targets.length) }, worker), + ); + return pages.flat(); + } + + async createResourceComment( + request: CreateResourceCommentRequest, + ): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready") { + throw new Error("Sign in to comment"); + } + return authStatus.auth.client.createResourceComment(request); + } + async getCloudRunArtifacts( taskId: string, runId: string, diff --git a/products/desktop/packages/shared/src/domain-types.ts b/products/desktop/packages/shared/src/domain-types.ts index 774f12ab4934..09dd3ca0578b 100644 --- a/products/desktop/packages/shared/src/domain-types.ts +++ b/products/desktop/packages/shared/src/domain-types.ts @@ -171,6 +171,8 @@ export type TaskActivityKind = | "completed" | "message" | "mention" + | "thread_reply" + | "owned_item_comment" | "created"; /** @@ -189,6 +191,9 @@ export interface TaskActivity { snippet: string; latest_author?: UserBasic | null; latest_message_id?: string | null; + latest_comment_id?: string | null; + latest_comment_scope?: string | null; + latest_comment_item_id?: string | null; is_unread: boolean; } @@ -203,6 +208,7 @@ export interface TaskActivityPage { export interface TaskActivityReadMarker { task_id: string; seen_before: string; + activity_id?: string; } export interface TaskActivityMarkReadResult { diff --git a/products/desktop/packages/shared/src/git-domain.ts b/products/desktop/packages/shared/src/git-domain.ts index 24cb09add12a..934652b4aed9 100644 --- a/products/desktop/packages/shared/src/git-domain.ts +++ b/products/desktop/packages/shared/src/git-domain.ts @@ -5,6 +5,7 @@ import { z } from "zod"; export const prReviewCommentUserSchema = z.object({ login: z.string(), avatar_url: z.string(), + isBot: z.boolean().optional(), }); export const prReviewCommentSchema = z.object({ @@ -156,6 +157,7 @@ export type GetPrChecksOutput = z.infer; export const prConversationCommentSchema = z.object({ id: z.number(), author: z.string(), + isBot: z.boolean().optional(), avatarUrl: z.string().nullable(), body: z.string(), createdAt: z.string(), diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityHoverCard.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityHoverCard.tsx index 82097a9e9723..87f102e2e1da 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ActivityHoverCard.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityHoverCard.tsx @@ -114,7 +114,7 @@ export function ActivityHoverCard({
{items.map((item) => ( diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.test.tsx new file mode 100644 index 000000000000..202ba3bca543 --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.test.tsx @@ -0,0 +1,170 @@ +import type { Task } from "@posthog/shared/domain-types"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { + afterEach, + beforeEach, + describe, + expect, + it, + type MockInstance, + vi, +} from "vitest"; + +vi.mock("@posthog/ui/features/canvas/hooks/useThreadConversation", () => ({ + useThreadConversation: () => ({ + timeline: [], + agentStatus: null, + events: [], + isPromptPending: false, + isReady: true, + members: [], + currentUser: null, + isTaskAuthor: true, + canForward: true, + draft: "", + setDraft: vi.fn(), + isSubmitDisabled: false, + submit: vi.fn(), + sendMessageToAgent: vi.fn(), + deleteMessage: vi.fn(), + onMentionInsert: vi.fn(), + }), +})); +vi.mock("@posthog/ui/features/canvas/components/ActivityTimeline", () => ({ + ActivityTimeline: () =>
timeline body
, +})); +vi.mock("@posthog/ui/features/canvas/components/TaskArtifactsList", () => ({ + TaskArtifactsList: () =>
artifacts body
, +})); +vi.mock("@posthog/ui/features/canvas/components/TaskCommentsList", () => ({ + TaskCommentsList: () =>
comments body
, +})); +vi.mock("@posthog/ui/features/canvas/components/ChannelFeedView", () => ({ + TaskCard: () =>
task card
, +})); +vi.mock("@posthog/ui/features/canvas/components/ThreadPanel", () => ({ + AgentStatusLine: () =>
agent status
, + ThreadLoadingState: () =>
loading
, + ThreadReplyComposer: () =>
composer
, +})); +vi.mock("@posthog/ui/features/tasks/queries", () => ({ + taskDetailQuery: () => ({ queryKey: ["task"], queryFn: vi.fn() }), +})); +vi.mock("@tanstack/react-query", () => ({ + useQuery: () => ({ data: undefined }), +})); +vi.mock("@posthog/ui/shell/analytics", () => ({ track: vi.fn() })); + +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; +import { ActivityPanel } from "./ActivityPanel"; + +const task = { id: "task-1", title: "Ship it" } as unknown as Task; + +function renderPanel(taskId = "task-1") { + return render( + , + ); +} + +describe("ActivityPanel", () => { + let scrollTo: MockInstance; + + beforeEach(() => { + scrollTo = vi.spyOn(Element.prototype, "scrollTo"); + useCommentNavigationStore.setState({ + focusByTask: {}, + resolutionsByTarget: {}, + }); + }); + + afterEach(() => { + scrollTo.mockRestore(); + }); + + it("offers comments as a third tab beside the timeline and artifacts", () => { + renderPanel(); + + expect(screen.getByRole("tab", { name: "Timeline" })).toBeTruthy(); + expect(screen.getByRole("tab", { name: "Artifacts" })).toBeTruthy(); + fireEvent.click(screen.getByRole("tab", { name: "Comments" })); + + expect(screen.getByText("comments body")).toBeTruthy(); + // The composer belongs to the conversation, not to a list of threads. + expect(screen.queryByText("composer")).toBeNull(); + }); + + // A thread picked on the artifact itself lands in this tab, so the pick has + // to bring the tab with it. + it("switches to comments when a thread is picked elsewhere", () => { + renderPanel(); + expect(screen.getByText("timeline body")).toBeTruthy(); + + act(() => + useCommentNavigationStore + .getState() + .requestCommentFocus( + "task-1", + { scope: "task_artifact", itemId: "artifact-1" }, + "comment-1", + ), + ); + + expect(screen.getByText("comments body")).toBeTruthy(); + }); + + it("leaves a focus request for another task alone", () => { + renderPanel(); + + act(() => + useCommentNavigationStore + .getState() + .requestCommentFocus( + "task-2", + { scope: "task_artifact", itemId: "artifact-1" }, + "comment-1", + ), + ); + + expect(screen.getByText("timeline body")).toBeTruthy(); + }); + + // A focus left over from an earlier visit must not hijack the panel, and the + // panel is reused across tasks without remounting. + it("does not open comments for a focus that predates the task", () => { + useCommentNavigationStore + .getState() + .requestCommentFocus( + "task-2", + { scope: "task_artifact", itemId: "artifact-1" }, + "comment-1", + ); + const { rerender } = renderPanel("task-1"); + + rerender( + , + ); + + expect(screen.getByText("timeline body")).toBeTruthy(); + }); + + // Only the timeline reads bottom-up; the thread lists put what matters on top. + it("keeps the comments list where it was scrolled to", () => { + renderPanel(); + expect(scrollTo).toHaveBeenCalled(); + const timelineScrolls = scrollTo.mock.calls.length; + + fireEvent.click(screen.getByRole("tab", { name: "Comments" })); + + expect(scrollTo.mock.calls.length).toBe(timelineScrolls); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.tsx index c59a26979b89..3d212e4cd56c 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityPanel.tsx @@ -9,23 +9,26 @@ import type { Task } from "@posthog/shared/domain-types"; import { ActivityTimeline } from "@posthog/ui/features/canvas/components/ActivityTimeline"; import { TaskCard } from "@posthog/ui/features/canvas/components/ChannelFeedView"; import { TaskArtifactsList } from "@posthog/ui/features/canvas/components/TaskArtifactsList"; +import { TaskCommentsList } from "@posthog/ui/features/canvas/components/TaskCommentsList"; import { AgentStatusLine, ThreadLoadingState, ThreadReplyComposer, } from "@posthog/ui/features/canvas/components/ThreadPanel"; import { useThreadConversation } from "@posthog/ui/features/canvas/hooks/useThreadConversation"; +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; import { buildConversationItems } from "@posthog/ui/features/sessions/components/buildConversationItems"; import { taskDetailQuery } from "@posthog/ui/features/tasks/queries"; import { track } from "@posthog/ui/shell/analytics"; import { useQuery } from "@tanstack/react-query"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -type ActivityTab = "timeline" | "artifacts"; +type ActivityTab = "timeline" | "artifacts" | "comments"; const ACTIVITY_TABS: readonly { key: ActivityTab; label: string }[] = [ { key: "timeline", label: "Timeline" }, { key: "artifacts", label: "Artifacts" }, + { key: "comments", label: "Comments" }, ] as const; /** The 32px row this panel leads with: the tabs are the header, so the strip @@ -161,15 +164,55 @@ function ActivityConversation({ [tab, events, isPromptPending], ); + // A thread picked on the artifact itself lives in the Comments tab, so the + // pick has to bring the tab with it. Only a fresh request switches tabs: a + // focus left over from an earlier visit must not hijack the panel on mount. + const commentFocus = useCommentNavigationStore( + (state) => state.focusByTask[taskId], + ); + const acknowledgeCommentsTabOpen = useCommentNavigationStore( + (state) => state.acknowledgeCommentsTabOpen, + ); + // Tracks the task too: this panel is reused across tasks without remounting, + // so a nonce seen for the previous task says nothing about this one. + const seenFocus = useRef<{ taskId: string; nonce: number | null }>({ + taskId, + nonce: null, + }); + useEffect(() => { + if (seenFocus.current.taskId !== taskId) { + seenFocus.current = { taskId, nonce: null }; + return; + } + if ( + commentFocus?.openCommentsTab && + commentFocus.nonce !== seenFocus.current.nonce + ) { + seenFocus.current = { taskId, nonce: commentFocus.nonce }; + // Not handleTabChange: a programmatic switch isn't a user tab change. + setTab("comments"); + } + }, [commentFocus, taskId]); + useEffect(() => { + if (tab === "comments" && commentFocus?.openCommentsTab) { + acknowledgeCommentsTabOpen(taskId, commentFocus.nonce); + } + }, [acknowledgeCommentsTabOpen, commentFocus, tab, taskId]); + const scrollRef = useRef(null); // biome-ignore lint/correctness/useExhaustiveDependencies: scroll when rendered thread content changes useEffect(() => { + // Only the timeline reads bottom-up; the other tabs put what matters on top. + if (tab !== "timeline") return; scrollRef.current?.scrollTo({ top: scrollRef.current.scrollHeight }); }, [timeline, events.length, agentStatus?.phase, tab]); const showComposer = tab === "timeline"; const body = () => { + if (tab === "comments") { + return ; + } if (tab === "artifacts") { return ( ({ + toChannelDashboard: vi.fn(), + toChannelTask: vi.fn(), + toTaskDetail: vi.fn(), +})); + +vi.mock("@posthog/ui/router/navigationBridge", () => ({ + navigateToChannelDashboard: navigation.toChannelDashboard, + navigateToChannelTask: navigation.toChannelTask, + navigateToTaskDetail: navigation.toTaskDetail, +})); +vi.mock("@posthog/ui/shell/analytics", () => ({ track: vi.fn() })); + +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; +import { ActivityRow, activityHeadline } from "./ActivityView"; function item(overrides: Partial): TaskActivityItem { return { @@ -21,6 +36,15 @@ function item(overrides: Partial): TaskActivityItem { } describe("activityHeadline", () => { + beforeEach(() => { + navigation.toChannelTask.mockReset(); + navigation.toChannelDashboard.mockReset(); + navigation.toTaskDetail.mockReset(); + useCommentNavigationStore.setState({ + focusByTask: {}, + resolutionsByTarget: {}, + }); + }); it.each([ [ "completed run", @@ -28,6 +52,33 @@ describe("activityHeadline", () => { "The agent completed this task", ], ["agent reply", item({ activityKind: "message" }), "The agent replied"], + [ + "thread reply", + item({ + activityKind: "thread_reply", + author: { + id: 2, + uuid: "author", + email: "author@posthog.com", + first_name: "Ann", + }, + }), + "replied to a thread you participated in", + ], + [ + "canvas owner comment", + item({ + activityKind: "owned_item_comment", + commentTarget: { scope: "desktop_canvas", itemId: "canvas-1" }, + author: { + id: 2, + uuid: "author", + email: "author@posthog.com", + first_name: "Ann", + }, + }), + "commented on your canvas", + ], [ "own reply", item({ @@ -59,4 +110,43 @@ describe("activityHeadline", () => { ); expect(getByText("#me")).toBeInTheDocument(); }); + + it("opens an activity mention at its exact comment thread", () => { + const activity = item({ + activityKind: "mention", + channelId: "channel-1", + commentId: "comment-1", + commentTarget: { scope: "desktop_canvas", itemId: "canvas-1" }, + author: { + id: 2, + uuid: "author", + email: "author@posthog.com", + first_name: "Ann", + }, + }); + + render( + , + ); + const activityButton = screen.getByText("mentioned you").closest("button"); + if (!activityButton) throw new Error("Expected activity row button"); + fireEvent.click(activityButton); + + expect(navigation.toChannelDashboard).toHaveBeenCalledWith( + "channel-1", + "canvas-1", + ); + expect(navigation.toChannelTask).not.toHaveBeenCalled(); + expect(useCommentNavigationStore.getState().focusByTask["task-1"]).toEqual({ + target: { scope: "desktop_canvas", itemId: "canvas-1" }, + threadId: "comment-1", + nonce: expect.any(Number), + openCommentsTab: true, + }); + }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityView.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityView.tsx index cd22d7c9364d..4871cf946fc0 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ActivityView.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityView.tsx @@ -28,8 +28,11 @@ import { MentionText } from "@posthog/ui/features/canvas/components/MentionText" import { useChannelsLayout } from "@posthog/ui/features/canvas/hooks/useChannelsLayout"; import { useMarkTaskActivityRead } from "@posthog/ui/features/canvas/hooks/useMarkTaskActivityRead"; import { useTaskActivity } from "@posthog/ui/features/canvas/hooks/useTaskActivity"; +import { useCanvasChatPanelStore } from "@posthog/ui/features/canvas/stores/canvasChatPanelStore"; +import { useThreadPanelStore } from "@posthog/ui/features/canvas/stores/threadPanelStore"; import { copyChannelLink } from "@posthog/ui/features/canvas/utils/copyChannelLink"; import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay"; +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; import { PageHeader, PageHeaderActions, @@ -40,6 +43,7 @@ import { PageHeaderTitleRow, } from "@posthog/ui/primitives/PageHeader"; import { + navigateToChannelDashboard, navigateToChannelTask, navigateToTaskDetail, } from "@posthog/ui/router/navigationBridge"; @@ -65,6 +69,17 @@ function ChannelSuffix({ channelName }: { channelName: string | null }) { ); } +function ownedItemName(item: TaskActivityItem): string { + switch (item.commentTarget?.scope) { + case "desktop_canvas": + return "canvas"; + case "task_artifact": + return "artifact"; + default: + return "task"; + } +} + /** The lead line describing what happened, chosen by the row's activity kind. */ export function activityHeadline( item: TaskActivityItem, @@ -112,6 +127,26 @@ export function activityHeadline( ); + case "thread_reply": + return ( + <> + + {userDisplayName(item.author)} + {" "} + replied to a thread you participated in + + + ); + case "owned_item_comment": + return ( + <> + + {userDisplayName(item.author)} + {" "} + commented on your {ownedItemName(item)} + + + ); default: return "You created this task"; } @@ -149,10 +184,23 @@ export function ActivityRow({ task_id: item.taskId, }); onOpen(item); + if (item.commentId && item.commentTarget) { + useCommentNavigationStore + .getState() + .requestCommentFocus(item.taskId, item.commentTarget, item.commentId); + } onNavigate?.(); + if (channelId && item.commentTarget?.scope === "desktop_canvas") { + useCanvasChatPanelStore.getState().openComments(); + navigateToChannelDashboard(channelId, item.commentTarget.itemId); + return; + } // The channel thread route is the deep-link target; unfiled tasks fall // back to the plain task view. if (channelId) { + if (item.commentId) { + useThreadPanelStore.getState().setCollapsed(false); + } navigateToChannelTask(channelId, item.taskId); } else { navigateToTaskDetail(item.taskId); @@ -270,7 +318,13 @@ export function ActivityView() { // reached any other way, so the feed converges either way. const markRead = useCallback( (item: TaskActivityItem) => - markTasksRead([{ task_id: item.taskId, seen_before: item.activityAt }]), + markTasksRead([ + { + task_id: item.taskId, + seen_before: item.activityAt, + ...(item.commentId ? { activity_id: item.id } : {}), + }, + ]), [markTasksRead], ); const markAllRead = useCallback(() => { @@ -311,8 +365,8 @@ export function ActivityView() { No activity yet - Tasks you create, get tagged in, or reply to across{" "} - {spacesLayout ? "spaces" : "channels"} land here. + Task updates and comment notifications across{" "} + {spacesLayout ? "spaces" : "channels"} appear here. @@ -320,7 +374,7 @@ export function ActivityView() {
{items.map((item) => ( - Tasks you're involved in across spaces. + Task updates and comment notifications across spaces. @@ -379,7 +433,7 @@ export function ActivityView() { Activity - Tasks you're involved in across{" "} + Task updates and comment notifications across{" "} {spacesLayout ? "spaces" : "channels"}.
diff --git a/products/desktop/packages/ui/src/features/canvas/components/MentionComposer.tsx b/products/desktop/packages/ui/src/features/canvas/components/MentionComposer.tsx index df0826b9945c..71b1b95dfdf6 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/MentionComposer.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/MentionComposer.tsx @@ -29,6 +29,8 @@ interface MentionComposerProps { placeholder?: string; rows?: number; inputClassName?: string; + /** Put the caret in the editor on mount, for a composer the user just opened. */ + autoFocus?: boolean; /** Rendered inside the input group after the editor (send button etc.). */ children?: ReactNode; } @@ -57,6 +59,7 @@ export function MentionComposer({ onValueChange, onSubmit, members, + autoFocus = false, allowAgentMention = false, onMentionInsert, placeholder, @@ -99,6 +102,7 @@ export function MentionComposer({ const editor = useEditor( { + autofocus: autoFocus ? "end" : false, extensions: [ StarterKit.configure({ heading: false, diff --git a/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx index cb0a58dc5671..c91f8062e533 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.test.tsx @@ -1,5 +1,5 @@ import type { Task, TaskRun, TaskRunArtifact } from "@posthog/shared"; -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, within } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ @@ -33,6 +33,36 @@ vi.mock("@posthog/ui/features/pr-review/usePrComments", () => ({ vi.mock("@posthog/ui/features/pr-review/usePrReviewThreads", () => ({ usePrReviewThreads: () => ({ data: undefined }), })); +vi.mock("@posthog/ui/features/sessions/components/useComments", () => ({ + useCommentsForTargetsQuery: () => ({ + data: [ + { + id: "comment-1", + source_comment: null, + item_id: "a", + content: "Tighten this summary", + created_at: "2024-01-01T00:00:00Z", + item_context: { anchor: { kind: "document" } }, + }, + { + id: "reply-1", + source_comment: "comment-1", + item_id: "a", + content: "Agreed", + created_at: "2024-01-01T00:01:00Z", + item_context: { anchor: { kind: "document" } }, + }, + { + id: "comment-2", + source_comment: null, + item_id: "a", + content: "Second thread", + created_at: "2024-01-01T00:02:00Z", + item_context: { anchor: { kind: "document" } }, + }, + ], + }), +})); import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; import { TaskArtifactsList } from "./TaskArtifactsList"; @@ -123,15 +153,29 @@ describe("TaskArtifactsList", () => { expect(screen.getByText("Pull request #2")).toBeTruthy(); }); - it("lists the files the agent uploaded, with their size", () => { + it("lists uploaded files with their comment count", () => { + mocks.runs = [ + run("run-1", { artifacts: [outputFile({ id: "a", size: 16861 })] }), + ]; + + render(); + + const row = screen.getByText("report.md").closest("button"); + expect(row).not.toBeNull(); + expect(within(row as HTMLElement).getByText("2")).toBeTruthy(); + expect(within(row as HTMLElement).queryByText(/File|KB/)).toBeNull(); + }); + + // The threads themselves live in the Comments tab now, so the pane must not + // grow a second list of them. + it("leaves the thread list to the Comments tab", () => { mocks.runs = [ run("run-1", { artifacts: [outputFile({ id: "a", size: 16861 })] }), ]; render(); - expect(screen.getByText("report.md")).toBeTruthy(); - expect(screen.getByText("File · 17 KB")).toBeTruthy(); + expect(screen.queryByText("Tighten this summary")).toBeNull(); }); // The row should read like the chat's file list: markdown looks like @@ -196,7 +240,7 @@ describe("TaskArtifactsList", () => { render(); expect(screen.getAllByText("report.md")).toHaveLength(1); - expect(screen.getByText("File · 2 KB")).toBeTruthy(); + expect(screen.queryByText(/File ·|KB/)).toBeNull(); }); it.each([ diff --git a/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx b/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx index c01cafd5a6f7..bd47eaedcb8f 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/TaskArtifactsList.tsx @@ -1,28 +1,25 @@ import { ArrowSquareOutIcon, + ChatCircleIcon, PackageIcon, SlackLogoIcon, } from "@phosphor-icons/react"; -import { - OUTPUT_ARTIFACT_TYPES, - parseRunArtifacts, - type RunArtifact, -} from "@posthog/core/canvas/runArtifactSchemas"; +import type { ResourceComment } from "@posthog/api-client/posthog-client"; import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; import { + Badge, Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle, } from "@posthog/quill"; -import { readPrUrls } from "@posthog/shared"; -import type { - Task, - TaskRun, - TaskThreadMessage, -} from "@posthog/shared/domain-types"; +import type { Task, TaskThreadMessage } from "@posthog/shared/domain-types"; import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; +import { + buildRows, + commentTargets, +} from "@posthog/ui/features/canvas/components/taskArtifactRows"; import { useTaskRuns } from "@posthog/ui/features/canvas/hooks/useTaskRuns"; import { canvasArtifactOpenHandler } from "@posthog/ui/features/canvas/utils/canvasArtifactNavigation"; import { openPrInReview } from "@posthog/ui/features/code-review/openPrInReview"; @@ -30,97 +27,13 @@ import { usePrArtifact } from "@posthog/ui/features/git-interaction/usePrArtifac import { usePanelLayoutStore } from "@posthog/ui/features/panels/panelLayoutStore"; import { usePrComments } from "@posthog/ui/features/pr-review/usePrComments"; import { usePrReviewThreads } from "@posthog/ui/features/pr-review/usePrReviewThreads"; +import { buildCommentThreads } from "@posthog/ui/features/sessions/components/commentViewTypes"; +import { useCommentsForTargetsQuery } from "@posthog/ui/features/sessions/components/useComments"; import { FileIcon } from "@posthog/ui/primitives/FileIcon"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; -import { formatFileSize } from "@posthog/ui/utils/formatFileSize"; import { type ReactNode, useMemo, useState } from "react"; -type ArtifactRow = - | { kind: "pr"; key: string; url: string } - | { kind: "canvas"; key: string; name: string; url: string | null } - | { - kind: "file"; - key: string; - artifactId: string | null; - name: string; - runId: string | null; - size: number | undefined; - } - | { kind: "slack"; key: string; url: string }; - -function readRunOutputs(run: TaskRun): RunArtifact[] { - return parseRunArtifacts( - (run as { artifacts?: unknown }).artifacts, - OUTPUT_ARTIFACT_TYPES, - ); -} - -function buildRows( - task: Task, - timeline: ThreadTimelineRow[], - runs: TaskRun[], -): ArtifactRow[] { - const rows: ArtifactRow[] = []; - const seenPrUrls = new Set(); - - const addPr = (url: string, key: string) => { - if (seenPrUrls.has(url)) return; - seenPrUrls.add(url); - rows.push({ kind: "pr", key, url }); - }; - - for (const row of timeline) { - if (row.kind !== "artifact") continue; - if (row.artifact.kind === "pr") { - addPr(row.artifact.url, row.message.id); - } else { - rows.push({ - kind: "canvas", - key: row.message.id, - name: row.artifact.name, - url: row.artifact.url, - }); - } - } - - const allRuns = - runs.length > 0 ? runs : task.latest_run ? [task.latest_run] : []; - - // Re-uploading a file replaces it rather than adding a second one: agents - // revise a deliverable and upload it again under the same name, so keeping - // every copy would bury the current one under its own drafts. - const newestByName = new Map(); - for (const run of allRuns) { - for (const outputPr of readPrUrls(run.output)) { - addPr(outputPr, `output-pr:${outputPr}`); - } - for (const file of readRunOutputs(run)) { - if (!file.name) continue; - const previous = newestByName.get(file.name); - const isNewer = - !previous || - (file.uploaded_at ?? "") >= (previous.file.uploaded_at ?? ""); - if (isNewer) newestByName.set(file.name, { file, runId: run.id }); - } - } - for (const [name, { file, runId }] of newestByName) { - rows.push({ - kind: "file", - key: `file:${file.id ?? file.storage_path ?? name}`, - artifactId: file.id ?? null, - name, - runId, - size: file.size, - }); - } - - const slackUrl = task.latest_run?.state?.slack_thread_url; - if (typeof slackUrl === "string" && slackUrl) { - rows.push({ kind: "slack", key: "slack-thread", url: slackUrl }); - } - - return rows; -} +const EMPTY_COMMENTS: ResourceComment[] = []; function ArtifactListRow({ icon, @@ -133,7 +46,7 @@ function ArtifactListRow({ }: { icon: ReactNode; title: string; - detail?: string | null; + detail?: ReactNode; external?: boolean; onOpen?: () => void; /** Renders a trailing button that leaves the app instead of opening the @@ -227,13 +140,30 @@ function PrRow({ ); } -function CanvasRow({ name, url }: { name: string; url: string | null }) { +function CanvasRow({ + name, + url, + commentCount, +}: { + name: string; + url: string | null; + commentCount: number; +}) { const open = canvasArtifactOpenHandler(url); return ( 0 ? ( + + + {commentCount} + + ) : ( + "Canvas" + ) + } onOpen={open} /> ); @@ -244,13 +174,14 @@ function FileRow({ runId, artifactId, name, - size, + commentCount, }: { taskId: string; runId: string | null; artifactId: string | null; name: string; - size: number | undefined; + /** Supplied by the pane's single comments query so each row doesn't fetch. */ + commentCount: number; }) { const openArtifactTab = usePanelLayoutStore((state) => state.openArtifactTab); const canOpen = !!runId && !!artifactId; @@ -267,7 +198,12 @@ function FileRow({ } title={name} - detail={["File", formatFileSize(size)].filter(Boolean).join(" · ")} + detail={ + + + {commentCount} + + } onOpen={onOpen} /> ); @@ -289,6 +225,22 @@ export function TaskArtifactsList({ () => buildRows(task, timeline, runs), [task, timeline, runs], ); + // One query for every row's badge, so N resources cost one request rather + // than one per row. The threads themselves live in the Comments tab. + const targets = useMemo(() => commentTargets(rows), [rows]); + const commentsQuery = useCommentsForTargetsQuery(targets, task.id); + const comments = commentsQuery.data ?? EMPTY_COMMENTS; + // Open threads only, so a row's badge agrees with what the Comments tab + // shows on the same resource. + const openCountByItem = useMemo(() => { + const counts = new Map(); + for (const thread of buildCommentThreads(comments)) { + const itemId = thread.root.item_id; + if (thread.resolved || !itemId) continue; + counts.set(itemId, (counts.get(itemId) ?? 0) + 1); + } + return counts; + }, [comments]); if (rows.length === 0) { return ( @@ -317,7 +269,14 @@ export function TaskArtifactsList({ openInPlaceTaskId={canOpenInPlace ? task.id : undefined} /> ) : row.kind === "canvas" ? ( - + ) : row.kind === "file" ? ( ) : ( ({ + runs: [] as TaskRun[], + comments: [] as unknown[], + activeArtifactId: null as string | null, + prConversation: [] as unknown[], + prReviewThreads: [] as unknown[], + openArtifactTab: vi.fn(), + openPrInReview: vi.fn(), + openExternalUrl: vi.fn(), + requestScrollToFile: vi.fn(), + prReply: vi.fn(async () => true), + prResolve: vi.fn(async () => true), + createComment: vi.fn(), + setResolved: vi.fn(), + createdFor: [] as unknown[], + resolvedFor: [] as unknown[], + queriedTargets: [] as unknown[], +})); + +function openThread(body: string): void { + const card = screen.getByText(body).closest("[data-comment-thread-id]"); + expect(card).not.toBeNull(); + fireEvent.click( + within(card as HTMLElement).getByRole("button", { + name: "Open comment thread", + }), + ); +} + +vi.mock("@posthog/ui/features/canvas/hooks/useTaskRuns", () => ({ + useTaskRuns: () => ({ runs: mocks.runs, isLoading: false }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useOrgMembers", () => ({ + useOrgMembers: () => ({ members: [] }), +})); +vi.mock("@posthog/ui/features/panels/panelLayoutStore", () => ({ + usePanelLayoutStore: () => mocks.openArtifactTab, + useActiveArtifactId: () => mocks.activeArtifactId, +})); +vi.mock("@posthog/ui/features/pr-review/usePrCommentsForUrls", () => ({ + usePrCommentsForUrls: (urls: string[]) => ({ + byUrl: new Map(urls.map((url) => [url, mocks.prConversation])), + isLoading: false, + }), +})); +vi.mock("@posthog/ui/features/pr-review/usePrReviewThreadsForUrls", () => ({ + usePrReviewThreadsForUrls: (urls: string[]) => ({ + byUrl: new Map(urls.map((url) => [url, mocks.prReviewThreads])), + isLoading: false, + }), +})); +vi.mock("@posthog/ui/features/git-interaction/usePrDetails", () => ({ + usePrTitles: () => ({}), +})); +vi.mock("@posthog/ui/shell/openExternal", () => ({ + openExternalUrl: (url: string) => mocks.openExternalUrl(url), +})); +// GitHub bodies render through MarkdownRenderer; the wiring under test is the +// list, not the markdown pipeline, so keep it to plain text here. +vi.mock("@posthog/ui/features/editor/components/MarkdownRenderer", () => ({ + MarkdownRenderer: ({ content }: { content: string }) => ( + {content} + ), +})); +vi.mock("@posthog/ui/features/code-review/openPrInReview", () => ({ + openPrInReview: (taskId: string, url: string) => + mocks.openPrInReview(taskId, url), +})); +vi.mock("@posthog/ui/features/code-review/reviewNavigationStore", () => ({ + useReviewNavigationStore: { + getState: () => ({ requestScrollToFile: mocks.requestScrollToFile }), + }, +})); +// Tiptap's editor renders no placeholder attribute and drags a lot of DOM into +// jsdom; the wiring under test is which target a composed comment posts to. +vi.mock("@posthog/ui/features/sessions/components/CommentComposer", () => ({ + CommentComposer: ({ + placeholder, + onSubmit, + }: { + placeholder: string; + onSubmit: (content: string, mentions: number[]) => void; + }) => ( + + ), +})); +vi.mock("@posthog/ui/features/code-review/hooks/usePrCommentActions", () => ({ + usePrCommentActions: () => ({ + reply: mocks.prReply, + resolve: mocks.prResolve, + }), +})); +vi.mock("@posthog/ui/features/sessions/components/useComments", () => ({ + useCommentsQuery: (target: unknown) => { + if (target) mocks.queriedTargets.push([target]); + return { + data: mocks.comments, + isLoading: false, + }; + }, + useCommentsForTargetsQuery: (targets: unknown) => { + if (Array.isArray(targets) && targets.length > 0) { + mocks.queriedTargets.push(targets); + } + return { + data: mocks.comments, + isLoading: false, + }; + }, + useCreateComment: (target: unknown) => { + mocks.createdFor.push(target); + return { mutateAsync: mocks.createComment, isPending: false }; + }, + useSetCommentResolved: (target: unknown) => { + mocks.resolvedFor.push(target); + return { mutate: mocks.setResolved, isPending: false }; + }, +})); + +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; +import { TaskCommentsList } from "./TaskCommentsList"; + +const task = { id: "task-1", latest_run: null } as unknown as Task; + +function run(artifacts: Partial[], id = "run-1"): TaskRun { + return { id, output: null, artifacts } as unknown as TaskRun; +} + +function outputFile( + overrides: Partial, +): Partial { + return { + type: "output", + name: "report.md", + storage_path: "runs/1/report.md", + ...overrides, + }; +} + +function prRun(url: string): TaskRun { + return { + id: "run-pr", + output: { pr_url: url }, + artifacts: [], + } as unknown as TaskRun; +} + +function reviewThread(overrides: Record = {}) { + return { + nodeId: "node-1", + isResolved: false, + rootId: 501, + filePath: "packages/ui/src/App.tsx", + comments: [ + { + id: 501, + body: "This needs a guard", + path: "packages/ui/src/App.tsx", + line: 12, + user: { login: "octocat", avatar_url: "" }, + created_at: "2024-01-02T00:00:00Z", + }, + ], + ...overrides, + }; +} + +function comment(overrides: Partial): ResourceComment { + return { + id: "comment-1", + created_by: null, + content: "Tighten this summary", + created_at: "2024-01-01T00:00:00Z", + item_id: "a", + item_context: { anchor: { kind: "document" } }, + scope: "task_artifact", + source_comment: null, + ...overrides, + } as ResourceComment; +} + +describe("TaskCommentsList", () => { + beforeEach(() => { + mocks.runs = [ + run([ + outputFile({ id: "a", name: "report.md" }), + outputFile({ + id: "b", + name: "summary.md", + storage_path: "runs/1/summary.md", + }), + ]), + ]; + mocks.comments = [ + comment({}), + comment({ + id: "reply-1", + source_comment: "comment-1", + content: "Agreed", + created_at: "2024-01-01T00:01:00Z", + }), + comment({ + id: "comment-2", + item_id: "b", + content: "Second thread", + created_at: "2024-01-01T00:02:00Z", + }), + ]; + mocks.activeArtifactId = null; + mocks.prConversation = []; + mocks.prReviewThreads = []; + mocks.openArtifactTab.mockReset(); + mocks.openPrInReview.mockReset(); + mocks.openExternalUrl.mockReset(); + mocks.requestScrollToFile.mockReset(); + mocks.prReply.mockClear(); + mocks.prResolve.mockClear(); + mocks.createComment.mockReset(); + mocks.createComment.mockResolvedValue({ id: "created-comment" }); + mocks.setResolved.mockReset(); + mocks.createdFor = []; + mocks.resolvedFor = []; + mocks.queriedTargets = []; + useCommentNavigationStore.setState({ + focusByTask: {}, + resolutionsByTarget: {}, + }); + }); + + it("queries and displays only the current canvas when restricted", async () => { + const onCanvasCommentOpen = vi.fn(); + mocks.comments = [ + comment({ + item_id: "canvas-1", + scope: "desktop_canvas", + content: "Canvas feedback", + item_context: { + anchor: { + kind: "text", + quote: "important copy", + prefix: "", + suffix: "", + start: 0, + end: 14, + }, + canvasVersionId: "version-2", + }, + }), + ]; + + render( + "V2"} + onCanvasCommentOpen={onCanvasCommentOpen} + />, + ); + + expect(mocks.queriedTargets.at(-1)).toEqual([ + { scope: "desktop_canvas", itemId: "canvas-1" }, + ]); + expect(screen.getByText("Canvas feedback")).toBeInTheDocument(); + expect(screen.getByText("“important copy”")).toBeInTheDocument(); + expect(screen.getByText("V2 ·")).toBeInTheDocument(); + expect(screen.queryByText("Selected text")).not.toBeInTheDocument(); + expect(screen.queryByText("Whole canvas")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("Filter by source")).not.toBeInTheDocument(); + expect(screen.queryByText("Launch canvas")).not.toBeInTheDocument(); + expect(mocks.createdFor.at(-1)).toEqual({ + scope: "desktop_canvas", + itemId: "canvas-1", + }); + + openThread("Canvas feedback"); + expect(onCanvasCommentOpen).toHaveBeenCalledWith("version-2"); + + await act(async () => { + fireEvent.click(screen.getByText(/Comment on this canvas/)); + }); + expect(mocks.createComment).toHaveBeenCalledWith({ + content: "Composed comment", + context: { + anchor: { kind: "document" }, + canvasVersionId: "version-2", + }, + mentions: [], + }); + }); + + it("shows selected artifact text alongside its source", () => { + mocks.comments = [ + comment({ + item_context: { + anchor: { + kind: "text", + quote: "Purpose", + prefix: "", + suffix: "", + start: 0, + end: 7, + }, + }, + }), + ]; + + render(); + + expect(screen.getByText("report.md")).toBeTruthy(); + expect(screen.getByText("“Purpose”")).toBeTruthy(); + }); + + it("loads canvas comments from a local-development artifact link", () => { + mocks.runs = []; + mocks.comments = [ + comment({ + item_id: "canvas-1", + scope: "desktop_canvas", + content: "Linked canvas feedback", + }), + ]; + + const timeline = [ + { + kind: "artifact", + timestamp: 1, + message: { id: "message-1" }, + artifact: { + kind: "canvas", + name: "Dev Joke Machine", + url: "http://localhost:8000/code/canvas/channel-1/canvas-1", + }, + }, + ] as unknown as ThreadTimelineRow[]; + + render(); + + expect(mocks.queriedTargets.at(-1)).toContainEqual({ + scope: "desktop_canvas", + itemId: "canvas-1", + }); + expect(screen.getByText("Linked canvas feedback")).toBeInTheDocument(); + }); + + // The tab is the one place to see every thread the task produced, so each row + // has to say which artifact it came from. + it("lists open threads from every artifact, newest first", () => { + render(); + + const newest = screen.getByText("Second thread"); + const oldest = screen.getByText("Tighten this summary"); + expect( + newest.compareDocumentPosition(oldest) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect(screen.getByText("summary.md")).toBeTruthy(); + expect(screen.getByText("report.md")).toBeTruthy(); + expect(screen.getByText(/1 reply/)).toBeTruthy(); + // The resolve/reopen reply is thread state, not a comment of its own. + expect(screen.queryByText("Agreed")).toBeTruthy(); + }); + + it("opens the artifact a thread belongs to and focuses that thread", () => { + render(); + + openThread("Tighten this summary"); + + expect(mocks.openArtifactTab).toHaveBeenCalledWith("task-1", { + runId: "run-1", + artifactId: "a", + name: "report.md", + }); + expect(useCommentNavigationStore.getState().focusByTask["task-1"]).toEqual({ + target: { scope: "task_artifact", itemId: "a" }, + threadId: "comment-1", + nonce: expect.any(Number), + openCommentsTab: true, + }); + }); + + // Clicking the same thread twice has to scroll twice, so every request is a + // new nonce rather than a no-op set. + it("re-requests focus for a thread already focused", () => { + render(); + + openThread("Tighten this summary"); + const first = useCommentNavigationStore.getState().focusByTask["task-1"]; + openThread("Tighten this summary"); + const second = useCommentNavigationStore.getState().focusByTask["task-1"]; + + expect(second?.nonce).toBeGreaterThan(first?.nonce ?? 0); + }); + + it("filters between open and resolved threads", () => { + mocks.comments = [ + comment({}), + comment({ + id: "state-1", + source_comment: "comment-1", + content: "Resolved this thread", + created_at: "2024-01-01T00:03:00Z", + item_context: { + anchor: { kind: "document" }, + threadState: "resolved", + }, + }), + comment({ + id: "comment-2", + item_id: "b", + content: "Second thread", + created_at: "2024-01-01T00:02:00Z", + }), + ]; + + render(); + + expect(screen.getByText("Second thread")).toBeTruthy(); + expect(screen.queryByText("Tighten this summary")).toBeNull(); + + fireEvent.click(screen.getByLabelText("Filter comments")); + fireEvent.click(screen.getByText("Resolved (1)")); + + expect(screen.getByText("Tighten this summary")).toBeTruthy(); + expect(screen.queryByText("Second thread")).toBeNull(); + }); + + it("warns when the anchored text the thread points at has changed", () => { + useCommentNavigationStore.setState({ + resolutionsByTarget: { + "task_artifact:a": new Map([["comment-1", "orphaned" as const]]), + }, + }); + + render(); + + expect(screen.getByText("The highlighted text changed")).toBeTruthy(); + }); + + it("replies and resolves against the thread's own resource", () => { + render(); + + // Each row builds its mutations from its own target, since the list spans + // several resources. + expect(mocks.createdFor).toContainEqual({ + scope: "task_artifact", + itemId: "a", + }); + expect(mocks.resolvedFor).toContainEqual({ + scope: "task_artifact", + itemId: "b", + }); + + const thread = screen + .getByText("Tighten this summary") + .closest("[data-comment-thread-id]") as HTMLElement; + fireEvent.click(within(thread).getByText("Resolve")); + + expect(mocks.setResolved).toHaveBeenCalledWith({ + root: expect.objectContaining({ id: "comment-1" }), + resolved: true, + }); + }); + + // The pane follows what's on screen, but a reader who picks a source owns the + // filter from then on. + it("narrows to the artifact open in the main pane", () => { + mocks.activeArtifactId = "b"; + + render(); + + expect(screen.getByText("Second thread")).toBeTruthy(); + expect(screen.queryByText("Tighten this summary")).toBeNull(); + }); + + it("stops following the main pane once a source is picked by hand", () => { + mocks.activeArtifactId = "b"; + const { rerender } = render(); + + fireEvent.click(screen.getByLabelText("Filter by source")); + fireEvent.click(screen.getByText(/^All sources/)); + mocks.activeArtifactId = "a"; + rerender(); + + expect(screen.getByText("Second thread")).toBeTruthy(); + expect(screen.getByText("Tighten this summary")).toBeTruthy(); + }); + + it("lists a PR's review threads and conversation comments", () => { + mocks.runs = [prRun("https://github.com/acme/repo/pull/7")]; + mocks.comments = []; + mocks.prReviewThreads = [reviewThread()]; + mocks.prConversation = [ + { + id: 900, + author: "octocat", + avatarUrl: null, + body: "Shipping this", + createdAt: "2024-01-03T00:00:00Z", + url: "https://github.com/acme/repo/pull/7#issuecomment-900", + }, + ]; + + render(); + + expect(screen.getByText("This needs a guard")).toBeTruthy(); + expect(screen.getByText("Shipping this")).toBeTruthy(); + expect(screen.getAllByText("PR #7").length).toBe(2); + // Only the file-anchored thread can be resolved on GitHub. + expect(screen.getAllByText("Resolve")).toHaveLength(1); + // The conversation comment can't be handled here, so it links out instead. + expect(screen.getByText("View on GitHub")).toBeTruthy(); + }); + + it("links a conversation comment out to GitHub", () => { + mocks.runs = [prRun("https://github.com/acme/repo/pull/7")]; + mocks.comments = []; + mocks.prConversation = [ + { + id: 900, + author: "octocat", + avatarUrl: null, + body: "Shipping this", + createdAt: "2024-01-03T00:00:00Z", + url: "https://github.com/acme/repo/pull/7#issuecomment-900", + }, + ]; + + render(); + fireEvent.click(screen.getByText("View on GitHub")); + + expect(mocks.openExternalUrl).toHaveBeenCalledWith( + "https://github.com/acme/repo/pull/7#issuecomment-900", + ); + }); + + it("opens a PR thread in the review pane at its file", () => { + mocks.runs = [prRun("https://github.com/acme/repo/pull/7")]; + mocks.comments = []; + mocks.prReviewThreads = [reviewThread()]; + + render(); + openThread("This needs a guard"); + + expect(mocks.openPrInReview).toHaveBeenCalledWith( + "task-1", + "https://github.com/acme/repo/pull/7", + ); + expect(mocks.requestScrollToFile).toHaveBeenCalledWith( + "task-1", + "packages/ui/src/App.tsx", + ); + }); + + it("replies and resolves a PR thread on GitHub", () => { + mocks.runs = [prRun("https://github.com/acme/repo/pull/7")]; + mocks.comments = []; + mocks.prReviewThreads = [reviewThread()]; + + render(); + const thread = screen + .getByText("This needs a guard") + .closest("[data-comment-thread-id]") as HTMLElement; + fireEvent.click(within(thread).getByText("Resolve")); + + expect(mocks.prResolve).toHaveBeenCalledWith("node-1", true); + expect(mocks.setResolved).not.toHaveBeenCalled(); + }); + + // Not every comment belongs to a deliverable; some are about the work. + it("posts a comment on the task itself", async () => { + render(); + + await act(async () => { + fireEvent.click(screen.getByText(/Comment on this task/)); + }); + + expect(mocks.createdFor).toContainEqual({ + scope: "task", + itemId: "task-1", + }); + expect(mocks.createComment).toHaveBeenCalledWith( + expect.objectContaining({ + content: "Composed comment", + context: { anchor: { kind: "document" } }, + }), + ); + }); + + it("shows an empty state pointing at the artifact surfaces", () => { + mocks.comments = []; + + render(); + + expect(screen.getByText("No open comments")).toBeTruthy(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/TaskCommentsList.tsx b/products/desktop/packages/ui/src/features/canvas/components/TaskCommentsList.tsx new file mode 100644 index 000000000000..906c30d74c0e --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/TaskCommentsList.tsx @@ -0,0 +1,663 @@ +import { + CaretDownIcon, + ChatCircleIcon, + FunnelSimpleIcon, + GitPullRequestIcon, +} from "@phosphor-icons/react"; +import type { ResourceComment } from "@posthog/api-client/posthog-client"; +import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; +import { commentTargetKey } from "@posthog/core/comments/anchors"; +import { + Button, + DropdownMenu, + DropdownMenuContent, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuTrigger, + Empty, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, + Spinner, +} from "@posthog/quill"; +import type { + Task, + TaskThreadMessage, + UserBasic, +} from "@posthog/shared/domain-types"; +import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; +import { + buildRows, + type CommentSource, + commentSources, + taskCommentTarget, +} from "@posthog/ui/features/canvas/components/taskArtifactRows"; +import { + byNewestActivity, + prCommentThreads, + resourceCommentThreads, + type SourceKind, + type TaskCommentThread, + threadSourceOptions, +} from "@posthog/ui/features/canvas/components/taskCommentThreads"; +import { useOrgMembers } from "@posthog/ui/features/canvas/hooks/useOrgMembers"; +import { useTaskRuns } from "@posthog/ui/features/canvas/hooks/useTaskRuns"; +import { canvasArtifactOpenHandler } from "@posthog/ui/features/canvas/utils/canvasArtifactNavigation"; +import { usePrCommentActions } from "@posthog/ui/features/code-review/hooks/usePrCommentActions"; +import { openPrInReview } from "@posthog/ui/features/code-review/openPrInReview"; +import { useReviewNavigationStore } from "@posthog/ui/features/code-review/reviewNavigationStore"; +import { usePrTitles } from "@posthog/ui/features/git-interaction/usePrDetails"; +import { + useActiveArtifactId, + usePanelLayoutStore, +} from "@posthog/ui/features/panels/panelLayoutStore"; +import { usePrCommentsForUrls } from "@posthog/ui/features/pr-review/usePrCommentsForUrls"; +import { usePrReviewThreadsForUrls } from "@posthog/ui/features/pr-review/usePrReviewThreadsForUrls"; +import { useCommentNavigationStore } from "@posthog/ui/features/sessions/commentNavigationStore"; +import { CommentComposer } from "@posthog/ui/features/sessions/components/CommentComposer"; +import { CommentThreadCard } from "@posthog/ui/features/sessions/components/CommentThreadCard"; +import type { HighlightResolution } from "@posthog/ui/features/sessions/components/commentViewTypes"; +import { readCommentContext } from "@posthog/ui/features/sessions/components/commentViewTypes"; +import { + useCommentsForTargetsQuery, + useCommentsQuery, + useCreateComment, + useSetCommentResolved, +} from "@posthog/ui/features/sessions/components/useComments"; +import { FileIcon } from "@posthog/ui/primitives/FileIcon"; +import { useEffect, useMemo, useRef, useState } from "react"; + +const EMPTY_COMMENTS: ResourceComment[] = []; +/** The whole task's threads in one request; slower than a single artifact's own + * poll because this one fans out across every resource. */ +const POLL_INTERVAL_MS = 30_000; +const PULSE_MS = 1_200; +const ALL_SOURCES = "all"; + +type StateFilter = "open" | "resolved"; + +/** The icon a source shows wherever it's named — the card label and the + * filter menu — so the two always agree. */ +function sourceIcon(kind: SourceKind, label: string, size = 12) { + switch (kind) { + case "pr": + return ( + + ); + case "canvas": + return iconForTemplate("", { size, className: "text-violet-9" }); + case "task": + return ; + default: + return ; + } +} + +function SourceLabel({ thread }: { thread: TaskCommentThread }) { + const replies = thread.entries.length - 1; + return ( + + {sourceIcon(thread.sourceKind, thread.sourceLabel)} + + {thread.sourceLabel} + + {thread.origin.kind === "pr-review" && ( + + · {thread.origin.filePath.split("/").at(-1)} + + )} + {replies > 0 && ( + + · {replies} {replies === 1 ? "reply" : "replies"} + + )} + + ); +} + +function TextCommentReference({ + root, + versionLabel, +}: { + root: ResourceComment; + versionLabel?: (versionId: string) => string | null; +}) { + const context = readCommentContext(root); + const version = context?.canvasVersionId + ? versionLabel?.(context.canvasVersionId) + : null; + const anchor = context?.anchor; + if (anchor?.kind !== "text") return null; + return ( + + {version && {version} ·} + + “{anchor.quote}” + + + ); +} + +/** + * A PostHog comment thread. Its own component so it can hold the mutations for + * its thread's resource — the list spans several, each with its own target. + */ +function ResourceThreadRow({ + thread, + source, + root, + taskId, + members, + selected, + pulsing, + resolution, + onOpen, + showSource = true, + commentVersionLabel, +}: { + thread: TaskCommentThread; + source: CommentSource; + root: ResourceComment; + taskId: string; + members: UserBasic[]; + selected: boolean; + pulsing: boolean; + resolution?: HighlightResolution; + onOpen: () => void; + showSource?: boolean; + commentVersionLabel?: (versionId: string) => string | null; +}) { + const createComment = useCreateComment(source.target, taskId); + const setResolved = useSetCommentResolved(source.target); + + return ( + + {showSource && } + + + } + onSelect={onOpen} + onReply={async (content, mentions) => { + await createComment.mutateAsync({ + content, + sourceCommentId: root.id, + context: readCommentContext(root) ?? { anchor: { kind: "document" } }, + mentions, + }); + }} + onResolve={(resolved) => setResolved.mutate({ root, resolved })} + /> + ); +} + +/** A GitHub thread. Reply and resolve go to GitHub, not to PostHog. */ +function PrThreadRow({ + thread, + selected, + pulsing, + onOpen, +}: { + thread: TaskCommentThread; + selected: boolean; + pulsing: boolean; + onOpen: () => void; +}) { + const origin = thread.origin; + const prUrl = origin.kind === "resource" ? null : origin.prUrl; + const { reply, resolve } = usePrCommentActions(prUrl); + const [busy, setBusy] = useState(false); + + const run = async (action: () => Promise) => { + setBusy(true); + try { + if (!(await action())) throw new Error("GitHub comment action failed"); + } finally { + setBusy(false); + } + }; + + return ( + } + // Only inline review threads accept replies and resolution; conversation + // comments are read here and linked out to GitHub to act on. + canReply={origin.kind === "pr-review"} + canResolve={origin.kind === "pr-review"} + viewHref={origin.kind === "pr-conversation" ? origin.url : undefined} + onSelect={onOpen} + onReply={(content) => + run(() => + origin.kind === "pr-review" + ? reply(origin.rootCommentId, content) + : Promise.resolve(false), + ) + } + onResolve={(resolved) => + run(() => + origin.kind === "pr-review" + ? resolve(origin.threadNodeId, resolved) + : Promise.resolve(false), + ) + } + /> + ); +} + +/** + * Every comment thread on the task: its artifacts, its canvases, its pull + * requests, and the task itself. Selecting one opens where it lives and locates + * it there, which is why no surface carries a thread list of its own. + */ +export function TaskCommentsList({ + task, + timeline, + onlySource, + canvasVersionId, + commentVersionLabel, + onCanvasCommentOpen, +}: { + task: Task; + timeline: ThreadTimelineRow[]; + /** Restricts the pane to one resource known by its host, without relying on + * the task timeline to rediscover it. */ + onlySource?: CommentSource; + canvasVersionId?: string | null; + commentVersionLabel?: (versionId: string) => string | null; + onCanvasCommentOpen?: (versionId: string | null) => void; +}) { + const { runs } = useTaskRuns(onlySource ? undefined : task.id); + const { members } = useOrgMembers(); + const openArtifactTab = usePanelLayoutStore((state) => state.openArtifactTab); + const activeArtifactId = useActiveArtifactId(task.id); + const requestCommentFocus = useCommentNavigationStore( + (state) => state.requestCommentFocus, + ); + const focus = useCommentNavigationStore( + (state) => state.focusByTask[task.id], + ); + const resolutionsByTarget = useCommentNavigationStore( + (state) => state.resolutionsByTarget, + ); + const [stateFilter, setStateFilter] = useState("open"); + const [sourceFilter, setSourceFilter] = useState(ALL_SOURCES); + const [pulseThreadId, setPulseThreadId] = useState(null); + const [draft, setDraft] = useState(""); + + const rows = useMemo( + () => buildRows(task, timeline, runs), + [task, timeline, runs], + ); + const sources = useMemo( + () => (onlySource ? [onlySource] : commentSources(task.id, rows)), + [task.id, rows, onlySource], + ); + const targets = useMemo( + () => sources.map((source) => source.target), + [sources], + ); + const singleSourceComments = useCommentsQuery( + onlySource?.target ?? null, + task.id, + ); + const taskComments = useCommentsForTargetsQuery( + onlySource ? [] : targets, + task.id, + { + live: true, + intervalMs: POLL_INTERVAL_MS, + }, + ); + const commentsQuery = onlySource ? singleSourceComments : taskComments; + const prUrls = useMemo( + () => + onlySource + ? [] + : rows.flatMap((row) => (row.kind === "pr" ? [row.url] : [])), + [rows, onlySource], + ); + const prConversation = usePrCommentsForUrls(prUrls); + const prReviews = usePrReviewThreadsForUrls(prUrls); + const prTitles = usePrTitles(prUrls); + + const taskTarget = useMemo(() => taskCommentTarget(task.id), [task.id]); + const composerTarget = onlySource?.target ?? taskTarget; + const composerSourceKey = commentTargetKey(composerTarget); + const createComment = useCreateComment(composerTarget, task.id); + + const threads = useMemo(() => { + const reviewByUrl = new Map(prReviews.byUrl); + const conversationByUrl = new Map(prConversation.byUrl); + const resourceThreads = resourceCommentThreads( + commentsQuery.data ?? EMPTY_COMMENTS, + sources, + ); + const prThreads = prUrls.flatMap((prUrl) => + prCommentThreads( + prUrl, + prTitles[prUrl] ?? `PR #${prUrl.split("/").at(-1)}`, + reviewByUrl.get(prUrl) ?? [], + conversationByUrl.get(prUrl) ?? [], + ), + ); + return [...resourceThreads, ...prThreads].sort(byNewestActivity); + }, [ + commentsQuery.data, + sources, + prUrls, + prTitles, + prReviews.byUrl, + prConversation.byUrl, + ]); + + // Every source that could ever hold a thread, whether or not it has one yet. + // Validating against this rather than the loaded threads lets the filter + // follow an artifact whose comments haven't arrived, and lets the task and + // PR sources stay selectable while empty. + const knownSourceKeys = useMemo(() => { + const keys = new Set( + sources.map((source) => commentTargetKey(source.target)), + ); + for (const prUrl of prUrls) keys.add(prUrl); + return keys; + }, [sources, prUrls]); + const sourceOptions = useMemo(() => threadSourceOptions(threads), [threads]); + const effectiveSourceFilter = + sourceFilter === ALL_SOURCES || knownSourceKeys.has(sourceFilter) + ? sourceFilter + : ALL_SOURCES; + const sourceLabel = + effectiveSourceFilter === ALL_SOURCES + ? "All sources" + : (sourceOptions.find((option) => option.key === effectiveSourceFilter) + ?.label ?? "All sources"); + + useEffect(() => { + if (sourceFilter !== effectiveSourceFilter) { + setSourceFilter(effectiveSourceFilter); + } + }, [effectiveSourceFilter, sourceFilter]); + + // Follow the artifact on screen until the reader picks a source themselves; + // after that the filter is theirs, not the pane's. + const sourceFilterTouched = useRef(false); + useEffect(() => { + if (onlySource || sourceFilterTouched.current) return; + setSourceFilter( + activeArtifactId + ? commentTargetKey({ scope: "task_artifact", itemId: activeArtifactId }) + : ALL_SOURCES, + ); + }, [activeArtifactId, onlySource]); + + const inSource = (thread: TaskCommentThread) => + effectiveSourceFilter === ALL_SOURCES || + thread.sourceKey === effectiveSourceFilter; + const scoped = threads.filter(inSource); + const openCount = scoped.filter((thread) => !thread.resolved).length; + const resolvedCount = scoped.length - openCount; + const visibleThreads = scoped.filter( + (thread) => thread.resolved === (stateFilter === "resolved"), + ); + + // A thread picked on the artifact itself has to surface here, even when a + // filter is hiding it. Each request is honoured once, by nonce: resolving the + // focused thread later must not drag the filters along with it. + const focusedThreadId = focus?.threadId ?? null; + const handledNonceRef = useRef(null); + useEffect(() => { + if (!focus || handledNonceRef.current === focus.nonce) return; + const focused = threads.find((thread) => thread.id === focus.threadId); + // The thread may still be loading, so wait rather than guess its filters. + if (!focused) return; + handledNonceRef.current = focus.nonce; + setStateFilter(focused.resolved ? "resolved" : "open"); + setSourceFilter((current) => + current === ALL_SOURCES || current === focused.sourceKey + ? current + : ALL_SOURCES, + ); + setPulseThreadId(focus.threadId); + requestAnimationFrame(() => { + document + .querySelector( + `[data-comment-thread-id="${CSS.escape(focus.threadId)}"]`, + ) + ?.scrollIntoView({ behavior: "smooth", block: "nearest" }); + }); + }, [focus, threads]); + // The pulse fades on its own; owning the timer in its own effect keeps it + // cleaned up on the next pulse or on unmount, without a stray ref. + useEffect(() => { + if (!pulseThreadId) return; + const timer = setTimeout(() => setPulseThreadId(null), PULSE_MS); + return () => clearTimeout(timer); + }, [pulseThreadId]); + + const openThread = (thread: TaskCommentThread) => { + const origin = thread.origin; + if (origin.kind === "pr-review" || origin.kind === "pr-conversation") { + openPrInReview(task.id, origin.prUrl); + if (origin.kind === "pr-review") { + // The review pane scrolls by file; a specific comment is as close as it + // gets until it grows a per-thread target. + useReviewNavigationStore + .getState() + .requestScrollToFile(task.id, origin.filePath); + } + return; + } + const { source, root } = origin; + if (source.kind === "canvas") { + requestCommentFocus(task.id, source.target, root.id); + if (onCanvasCommentOpen) { + onCanvasCommentOpen(readCommentContext(root)?.canvasVersionId ?? null); + return; + } + canvasArtifactOpenHandler(source.url)?.(); + return; + } + // A thread on the task itself has nowhere else to open — it lives here. + if (source.kind === "task" || !source.runId) return; + openArtifactTab(task.id, { + runId: source.runId, + artifactId: source.target.itemId, + name: source.name, + }); + requestCommentFocus(task.id, source.target, root.id); + }; + + const loading = + commentsQuery.isLoading || prConversation.isLoading || prReviews.isLoading; + + return ( + // The parent scrolls the middle; the filters and the composer are pinned so + // they stay reachable however long the thread list grows. +
+
+ {!onlySource && ( + + + {sourceLabel} + + + } + /> + {/* Wide, single-line rows: the label truncates at the end (with the + full name on hover) and the count is pinned right with the shared + ml-auto idiom, so a long PR title stays legible and aligned. */} + + { + sourceFilterTouched.current = true; + setSourceFilter(value); + }} + > + + + All sources + + {threads.length} + + + {sourceOptions.map((option) => ( + + {sourceIcon(option.kind, option.label)} + {option.label} + + {option.count} + + + ))} + + + + )} + + + {stateFilter === "open" ? "Open" : "Resolved"} + + + } + /> + + setStateFilter(value as StateFilter)} + > + + Open ({openCount}) + + + Resolved ({resolvedCount}) + + + + +
+
+ {loading && threads.length === 0 ? ( +
+ +
+ ) : visibleThreads.length === 0 ? ( + + + + + + + No {stateFilter === "open" ? "open" : "resolved"} comments + + + {stateFilter === "open" + ? onlySource + ? "Comment on this canvas to start a thread." + : "Comment on the task below, or open an artifact and select text to start a thread there." + : "Resolved threads will appear here."} + + + + ) : ( + visibleThreads.map((thread) => + thread.origin.kind === "resource" ? ( + openThread(thread)} + showSource={!onlySource} + commentVersionLabel={commentVersionLabel} + /> + ) : ( + openThread(thread)} + /> + ), + ) + )} +
+
+ { + await createComment.mutateAsync({ + content, + context: { + anchor: { kind: "document" }, + ...(canvasVersionId ? { canvasVersionId } : {}), + }, + mentions, + }); + setDraft(""); + // Show the thread that was just opened: open state, and a source + // filter that isn't hiding the task's own comments. + setStateFilter("open"); + if ( + sourceFilter !== ALL_SOURCES && + sourceFilter !== composerSourceKey + ) { + setSourceFilter(ALL_SOURCES); + } + }} + members={members} + placeholder={`Comment on this ${onlySource ? "canvas" : "task"}… Type @ to mention someone`} + rows={2} + disabled={createComment.isPending} + /> +
+
+ ); +} diff --git a/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.test.tsx index a0017fb932c8..f81e49815b74 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.test.tsx @@ -1,19 +1,25 @@ import { Theme } from "@radix-ui/themes"; -import { render, screen } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; vi.mock("@posthog/ui/features/canvas/hooks/useChannelsLayout", () => ({ useChannelsLayout: () => false, })); +vi.mock("@posthog/host-router/react", () => ({ + useHostTRPC: () => ({ + dashboards: { saveContext: { mutationKey: () => ["save-context"] } }, + }), +})); -const { useChannelTasks, useParams, usePathname, useTasks } = vi.hoisted( - () => ({ +const { useChannelTasks, useDashboard, useParams, usePathname, useTasks } = + vi.hoisted(() => ({ useChannelTasks: vi.fn(), + useDashboard: vi.fn(), useParams: vi.fn(), usePathname: vi.fn(), useTasks: vi.fn(), - }), -); + })); vi.mock("@tanstack/react-router", () => ({ Outlet: () => null, @@ -45,8 +51,35 @@ vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ }), })); vi.mock("@posthog/ui/features/canvas/hooks/useDashboards", () => ({ - useDashboard: () => ({ dashboard: undefined }), + useDashboard, useDashboardMutations: () => ({}), + useCanvasVersions: () => ({ versions: [{ taskId: "version-task" }] }), +})); +vi.mock("@posthog/ui/features/sessions/components/useComments", () => ({ + useCommentsQuery: () => ({ + data: [ + { + id: "comment-1", + created_at: "2026-01-01T00:00:00Z", + content: "First", + item_id: "canvas-1", + item_context: { anchor: { kind: "document" } }, + scope: "desktop_canvas", + source_comment: null, + completed_at: null, + }, + { + id: "comment-2", + created_at: "2026-01-01T00:01:00Z", + content: "Second", + item_id: "canvas-1", + item_context: { anchor: { kind: "document" } }, + scope: "desktop_canvas", + source_comment: null, + completed_at: null, + }, + ], + }), })); vi.mock("@posthog/ui/features/canvas/stores/dashboardEditStore", () => ({ useDashboardEditStore: (sel: (s: unknown) => unknown) => @@ -60,6 +93,7 @@ vi.mock("@posthog/ui/features/canvas/freeform/CanvasFrameHost", () => ({ CanvasFrameHost: () => null, })); +import { useCanvasChatPanelStore } from "@posthog/ui/features/canvas/stores/canvasChatPanelStore"; import { useHeaderStore } from "@posthog/ui/shell/headerStore"; import { WebsiteLayout } from "./WebsiteLayout"; @@ -68,11 +102,17 @@ function renderLayout({ params, tasks = [{ id: "task-1", title: "Fix the bug" }], channelTaskIds = tasks.map((task) => task.id), + dashboard, }: { pathname: string; params: Record; tasks?: { id: string; title: string }[]; channelTaskIds?: string[]; + dashboard?: { + name: string; + templateId: string; + generationTaskId: string | null; + }; }) { usePathname.mockReturnValue(pathname); useParams.mockReturnValue(params); @@ -81,15 +121,46 @@ function renderLayout({ tasks: channelTaskIds.map((taskId) => ({ taskId })), isLoading: false, }); + useDashboard.mockReturnValue({ dashboard }); useHeaderStore.setState({ content: crumb }); render( - - - , + + + + + , ); } describe("WebsiteLayout task header actions", () => { + it.each([ + ["while generating", "task-1"], + ["after generation", null], + ])( + "shows the comment count and opens comments %s", + (_label, generationTaskId) => { + renderLayout({ + pathname: "/website/chan-1/dashboards/canvas-1", + params: { channelId: "chan-1", dashboardId: "canvas-1" }, + dashboard: { + name: "Launch", + templateId: "freeform", + generationTaskId, + }, + }); + + expect( + screen.getByRole("button", { name: /Comments/ }), + ).toHaveTextContent("Comments2"); + useCanvasChatPanelStore.setState({ collapsed: true, tab: "chat" }); + fireEvent.click(screen.getByRole("button", { name: /Comments/ })); + expect(useCanvasChatPanelStore.getState()).toMatchObject({ + collapsed: false, + tab: "comments", + }); + }, + ); + it("renders the task action row on a channel task detail", () => { renderLayout({ pathname: "/website/chan-1/tasks/task-1", diff --git a/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.tsx b/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.tsx index 2a0045508ae6..44e5012be01b 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/WebsiteLayout.tsx @@ -1,5 +1,6 @@ import { ArrowClockwiseIcon, + ChatCircleIcon, DotsThreeIcon, LinkIcon, PencilSimpleIcon, @@ -25,14 +26,22 @@ import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; import { useChannelsLayout } from "@posthog/ui/features/canvas/hooks/useChannelsLayout"; import { useChannelTasks } from "@posthog/ui/features/canvas/hooks/useChannelTasks"; import { + useCanvasVersions, useDashboard, useDashboardMutations, } from "@posthog/ui/features/canvas/hooks/useDashboards"; +import { useCanvasChatPanelStore } from "@posthog/ui/features/canvas/stores/canvasChatPanelStore"; import { useDashboardEditStore, useIsDashboardEditing, } from "@posthog/ui/features/canvas/stores/dashboardEditStore"; import { copyCanvasLink } from "@posthog/ui/features/canvas/utils/copyCanvasLink"; +import { buildCommentThreads } from "@posthog/ui/features/sessions/components/commentViewTypes"; +import { useCommentsQuery } from "@posthog/ui/features/sessions/components/useComments"; +import { + MentionAvailabilityProvider, + PRIVATE_SPACE_MENTIONS_DISABLED, +} from "@posthog/ui/features/sessions/mentionAvailability"; import { TaskHeaderActions } from "@posthog/ui/features/task-detail/components/TaskHeaderActions"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; import { toast } from "@posthog/ui/primitives/toast"; @@ -195,8 +204,26 @@ function CanvasBreadcrumb({ trailing?: ReactNode; }) { const { dashboard } = useDashboard(dashboardId); + const { versions } = useCanvasVersions(dashboardId); const { renameDashboard } = useDashboardMutations(); + const openComments = useCanvasChatPanelStore((state) => state.openComments); const name = dashboard?.name ?? "Canvas"; + const commentTarget = { + scope: "desktop_canvas" as const, + itemId: dashboardId, + }; + const commentTaskId = + dashboard?.generationTaskId ?? + versions.find((version) => version.taskId)?.taskId ?? + null; + const comments = useCommentsQuery( + commentTaskId ? commentTarget : null, + commentTaskId ?? "", + { live: true }, + ); + const openCommentCount = buildCommentThreads(comments.data ?? []).filter( + (thread) => !thread.resolved, + ).length; return ( void renameDashboard(dashboardId, next)} - trailing={trailing} + trailing={ + <> + {commentTaskId && ( + + )} + {trailing} + + } /> ); } @@ -242,6 +282,11 @@ export function WebsiteLayout() { : undefined; const { channels } = useChannels(); + const mentionsDisabledReason = + channels.find((channel) => channel.id === channelId)?.channelType === + "personal" + ? PRIVATE_SPACE_MENTIONS_DISABLED + : null; const channelName = channelId ? (channels.find((c) => c.id === channelId)?.name ?? (spacesLayout ? "Space" : "Channel")) @@ -314,7 +359,9 @@ export function WebsiteLayout() { )} - + + + {/* Warm-iframe pool for canvases. Mounted once here so it persists across every in-space navigation; overlays itself onto the active canvas's diff --git a/products/desktop/packages/ui/src/features/canvas/components/activityFeed.ts b/products/desktop/packages/ui/src/features/canvas/components/activityFeed.ts index 8cb1d13cd415..a0867a55d56b 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/activityFeed.ts +++ b/products/desktop/packages/ui/src/features/canvas/components/activityFeed.ts @@ -10,6 +10,7 @@ export function activityReadPayload(items: TaskActivityItem[]) { return items.map((item) => ({ task_id: item.taskId, seen_before: item.activityAt, + ...(item.commentId ? { activity_id: item.id } : {}), })); } diff --git a/products/desktop/packages/ui/src/features/canvas/components/taskArtifactRows.ts b/products/desktop/packages/ui/src/features/canvas/components/taskArtifactRows.ts new file mode 100644 index 000000000000..a44423b591ff --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/taskArtifactRows.ts @@ -0,0 +1,199 @@ +import { + OUTPUT_ARTIFACT_TYPES, + parseRunArtifacts, + type RunArtifact, +} from "@posthog/core/canvas/runArtifactSchemas"; +import type { ThreadTimelineRow } from "@posthog/core/canvas/threadTimeline"; +import { + type CommentTarget, + commentTargetKey, +} from "@posthog/core/comments/anchors"; +import { readPrUrls } from "@posthog/shared"; +import type { + Task, + TaskRun, + TaskThreadMessage, +} from "@posthog/shared/domain-types"; +import { parseHttpsUrl, parseShareLink } from "@posthog/ui/utils/posthogLinks"; + +export type ArtifactRow = + | { kind: "pr"; key: string; url: string } + | { + kind: "canvas"; + key: string; + name: string; + url: string | null; + /** The canvas row id, the stable comment target (never the name). */ + dashboardId: string | null; + } + | { + kind: "file"; + key: string; + artifactId: string | null; + name: string; + runId: string | null; + } + | { kind: "slack"; key: string; url: string }; + +/** + * Somewhere a task's comment threads live. Artifacts and canvases come from the + * task's rows; the task itself is always one, holding the threads that belong + * to the work rather than to any single deliverable. + */ +export type CommentSource = + | { kind: "file"; target: CommentTarget; name: string; runId: string | null } + | { kind: "canvas"; target: CommentTarget; name: string; url: string | null } + | { kind: "task"; target: CommentTarget; name: string }; + +export function taskCommentTarget(taskId: string): CommentTarget { + return { scope: "task", itemId: taskId }; +} + +export function commentSources( + taskId: string, + rows: ArtifactRow[], +): CommentSource[] { + const sources: CommentSource[] = [ + { kind: "task", target: taskCommentTarget(taskId), name: "This task" }, + ]; + const seen = new Set(); + for (const row of rows) { + const target = targetForRow(row); + if (!target || seen.has(commentTargetKey(target))) continue; + seen.add(commentTargetKey(target)); + if (row.kind === "file") { + sources.push({ kind: "file", target, name: row.name, runId: row.runId }); + } else if (row.kind === "canvas") { + sources.push({ kind: "canvas", target, name: row.name, url: row.url }); + } + } + return sources; +} + +/** The canvas's stable row id, recovered from its share link. */ +function canvasDashboardId(url: string | null): string | null { + if (!url) return null; + const parsed = parseHttpsUrl(url); + const target = parsed ? parseShareLink(parsed.href) : null; + if (target?.kind === "canvas") return target.dashboardId; + + // Local development emits http:// canvas links, which are deliberately not + // valid external share links. Recover only the exact route's final id here; + // this value is used for an access-checked API query, never for navigation. + try { + const localUrl = new URL(url); + if (localUrl.protocol !== "http:") return null; + const segments = localUrl.pathname.split("/").filter(Boolean); + if ( + segments.length === 4 && + segments[0] === "code" && + segments[1] === "canvas" + ) { + return decodeURIComponent(segments[3]); + } + } catch { + return null; + } + return null; +} + +/** Where a row's comments live, or null when the row can't carry any. */ +function targetForRow(row: ArtifactRow): CommentTarget | null { + if (row.kind === "file" && row.artifactId) { + return { scope: "task_artifact", itemId: row.artifactId }; + } + if (row.kind === "canvas" && row.dashboardId) { + return { scope: "desktop_canvas", itemId: row.dashboardId }; + } + return null; +} + +/** + * Every commentable resource this task produced, once each. Artifacts and + * canvases share the generic comments API, differing only by scope, so a pane + * can hold one query over all of them — and two timeline messages naming the + * same canvas must not fetch it twice. + */ +export function commentTargets(rows: ArtifactRow[]): CommentTarget[] { + const byKey = new Map(); + for (const row of rows) { + const target = targetForRow(row); + if (target) byKey.set(commentTargetKey(target), target); + } + return [...byKey.values()]; +} + +function readRunOutputs(run: TaskRun): RunArtifact[] { + return parseRunArtifacts( + (run as { artifacts?: unknown }).artifacts, + OUTPUT_ARTIFACT_TYPES, + ); +} + +export function buildRows( + task: Task, + timeline: ThreadTimelineRow[], + runs: TaskRun[], +): ArtifactRow[] { + const rows: ArtifactRow[] = []; + const seenPrUrls = new Set(); + + const addPr = (url: string, key: string) => { + if (seenPrUrls.has(url)) return; + seenPrUrls.add(url); + rows.push({ kind: "pr", key, url }); + }; + + for (const row of timeline) { + if (row.kind !== "artifact") continue; + if (row.artifact.kind === "pr") { + addPr(row.artifact.url, row.message.id); + } else { + const url = row.artifact.url; + rows.push({ + kind: "canvas", + key: row.message.id, + name: row.artifact.name, + url, + dashboardId: canvasDashboardId(url), + }); + } + } + + const allRuns = + runs.length > 0 ? runs : task.latest_run ? [task.latest_run] : []; + + // Re-uploading a file replaces it rather than adding a second one: agents + // revise a deliverable and upload it again under the same name, so keeping + // every copy would bury the current one under its own drafts. + const newestByName = new Map(); + for (const run of allRuns) { + for (const outputPr of readPrUrls(run.output)) { + addPr(outputPr, `output-pr:${outputPr}`); + } + for (const file of readRunOutputs(run)) { + if (!file.name) continue; + const previous = newestByName.get(file.name); + const isNewer = + !previous || + (file.uploaded_at ?? "") >= (previous.file.uploaded_at ?? ""); + if (isNewer) newestByName.set(file.name, { file, runId: run.id }); + } + } + for (const [name, { file, runId }] of newestByName) { + rows.push({ + kind: "file", + key: `file:${file.id ?? file.storage_path ?? name}`, + artifactId: file.id ?? null, + name, + runId, + }); + } + + const slackUrl = task.latest_run?.state?.slack_thread_url; + if (typeof slackUrl === "string" && slackUrl) { + rows.push({ kind: "slack", key: "slack-thread", url: slackUrl }); + } + + return rows; +} diff --git a/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.test.ts b/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.test.ts new file mode 100644 index 000000000000..624d06996929 --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.test.ts @@ -0,0 +1,260 @@ +import type { ResourceComment } from "@posthog/api-client/posthog-client"; +import type { PrConversationComment, PrReviewThread } from "@posthog/shared"; +import { describe, expect, it } from "vitest"; +import type { CommentSource } from "./taskArtifactRows"; +import { + byNewestActivity, + prCommentThreads, + resourceCommentThreads, + threadSourceOptions, +} from "./taskCommentThreads"; + +const fileSource: CommentSource = { + kind: "file", + target: { scope: "task_artifact", itemId: "a" }, + name: "report.md", + runId: "run-1", +}; +const taskSource: CommentSource = { + kind: "task", + target: { scope: "task", itemId: "task-1" }, + name: "This task", +}; + +function comment(overrides: Partial): ResourceComment { + return { + id: "c1", + created_by: null, + content: "hi", + created_at: "2024-01-01T00:00:00Z", + item_id: "a", + item_context: { anchor: { kind: "document" } }, + scope: "task_artifact", + source_comment: null, + ...overrides, + } as ResourceComment; +} + +describe("resourceCommentThreads", () => { + it("keeps only threads whose resource is present, tagged with it", () => { + const threads = resourceCommentThreads( + [ + comment({ id: "c1", item_id: "a", content: "root" }), + comment({ + id: "r1", + item_id: "a", + source_comment: "c1", + content: "reply", + created_at: "2024-01-01T00:01:00Z", + }), + comment({ id: "orphan", item_id: "gone", content: "no source" }), + ], + [fileSource, taskSource], + ); + + expect(threads).toHaveLength(1); + expect(threads[0].sourceKind).toBe("file"); + expect(threads[0].sourceLabel).toBe("report.md"); + expect(threads[0].entries.map((entry) => entry.body)).toEqual([ + "root", + "reply", + ]); + }); + + // A resolve/reopen reply is thread state, not something anyone said. + it("drops thread-state replies from the visible entries", () => { + const threads = resourceCommentThreads( + [ + comment({ id: "c1", content: "root" }), + comment({ + id: "state", + source_comment: "c1", + content: "Resolved this thread", + created_at: "2024-01-01T00:02:00Z", + item_context: { + anchor: { kind: "document" }, + threadState: "resolved", + }, + }), + ], + [fileSource], + ); + + expect(threads[0].resolved).toBe(true); + expect(threads[0].entries).toHaveLength(1); + }); +}); + +describe("prCommentThreads", () => { + const reviewThread: PrReviewThread = { + nodeId: "node-1", + isResolved: true, + rootId: 501, + filePath: "src/App.tsx", + comments: [ + { + id: 501, + body: "root", + path: "src/App.tsx", + line: 3, + original_line: null, + side: "RIGHT", + start_line: null, + start_side: null, + diff_hunk: "", + user: { login: "octo", avatar_url: "http://x/a.png" }, + created_at: "2024-01-01T00:00:00Z", + updated_at: "2024-01-01T00:00:00Z", + subject_type: "line", + }, + { + id: 502, + body: "reply", + path: "src/App.tsx", + line: 3, + original_line: null, + side: "RIGHT", + start_line: null, + start_side: null, + diff_hunk: "", + user: { login: "octo", avatar_url: "" }, + created_at: "2024-01-01T00:05:00Z", + updated_at: "2024-01-01T00:05:00Z", + subject_type: "line", + }, + ], + }; + const conversation: PrConversationComment = { + id: 900, + author: "octo", + avatarUrl: null, + body: "lgtm", + createdAt: "2024-01-02T00:00:00Z", + url: "https://github.com/a/b/pull/7#c", + }; + + it("carries review-thread replies, resolution and the reply/resolve ids", () => { + const [thread] = prCommentThreads("url", "PR #7", [reviewThread], []); + + expect(thread.entries.map((entry) => entry.body)).toEqual([ + "root", + "reply", + ]); + expect(thread.resolved).toBe(true); + expect(thread.origin).toMatchObject({ + kind: "pr-review", + rootCommentId: 501, + threadNodeId: "node-1", + filePath: "src/App.tsx", + }); + expect(thread.lastActivityAt).toBe("2024-01-01T00:05:00Z"); + }); + + it("makes each conversation comment its own unresolvable thread", () => { + const [thread] = prCommentThreads("url", "PR #7", [], [conversation]); + + expect(thread.entries).toHaveLength(1); + expect(thread.resolved).toBe(false); + expect(thread.origin.kind).toBe("pr-conversation"); + }); + + it("omits GitHub bot comments without hiding human threads", () => { + const botRoot = { + ...reviewThread, + nodeId: "bot-root", + comments: reviewThread.comments.map((comment) => ({ + ...comment, + user: { ...comment.user, isBot: true }, + })), + }; + const humanRootWithBotReply = { + ...reviewThread, + nodeId: "human-root", + comments: [ + reviewThread.comments[0], + { + ...reviewThread.comments[1], + user: { ...reviewThread.comments[1].user, isBot: true }, + }, + ], + }; + + const threads = prCommentThreads( + "url", + "PR #7", + [botRoot, humanRootWithBotReply], + [conversation, { ...conversation, id: 901, isBot: true }], + ); + + expect(threads).toHaveLength(2); + expect( + threads.map((thread) => thread.entries.map((entry) => entry.body)), + ).toEqual([["root"], ["lgtm"]]); + }); +}); + +describe("threadSourceOptions / byNewestActivity", () => { + it("lists each source once and sorts newest first", () => { + const threads = [ + ...resourceCommentThreads( + [comment({ id: "c1", content: "old" })], + [fileSource], + ), + ...prCommentThreads( + "url", + "PR #7", + [], + [ + { + id: 1, + author: "octo", + avatarUrl: null, + body: "new", + createdAt: "2025-01-01T00:00:00Z", + url: null, + }, + ], + ), + ].sort(byNewestActivity); + + expect(threads[0].entries[0].body).toBe("new"); + // Options follow list order, which is newest-first, and carry the kind so + // the filter can show a matching icon. + expect( + threadSourceOptions(threads).map((option) => [option.label, option.kind]), + ).toEqual([ + ["PR #7", "pr"], + ["report.md", "file"], + ]); + }); + + // The task is the one source every task has, so it sits at the top of the + // filter regardless of when it was last touched. + it("pins the task source first, keeping the rest newest-first", () => { + const threads = [ + ...resourceCommentThreads( + [ + comment({ + id: "f1", + item_id: "a", + content: "file", + created_at: "2025-01-01T00:00:00Z", + }), + comment({ + id: "t1", + item_id: "task-1", + scope: "task", + content: "task", + created_at: "2024-01-01T00:00:00Z", + }), + ], + [fileSource, taskSource], + ), + ].sort(byNewestActivity); + + expect(threadSourceOptions(threads).map((option) => option.kind)).toEqual([ + "task", + "file", + ]); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.ts b/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.ts new file mode 100644 index 000000000000..0dbb40675d3a --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/taskCommentThreads.ts @@ -0,0 +1,228 @@ +import type { ResourceComment } from "@posthog/api-client/posthog-client"; +import { commentTargetKey } from "@posthog/core/comments/anchors"; +import type { PrConversationComment, PrReviewThread } from "@posthog/shared"; +import type { UserBasic } from "@posthog/shared/domain-types"; +import type { CommentSource } from "@posthog/ui/features/canvas/components/taskArtifactRows"; +import { + buildCommentThreads, + readCommentContext, +} from "@posthog/ui/features/sessions/components/commentViewTypes"; + +export type CommentEntry = { + id: string; + authorName: string; + /** A PostHog author, whose avatar and hue are the ones they have app-wide. */ + user: UserBasic | null; + /** A GitHub author, who only comes with an avatar url. */ + avatarUrl: string | null; + createdAt: string; + body: string; + /** PostHog comments carry @mention markup; GitHub bodies are markdown. */ + format: "mentions" | "markdown"; +}; + +/** How a thread is opened, replied to and resolved — one case per backend. */ +export type ThreadOrigin = + | { + kind: "resource"; + source: CommentSource; + /** The root comment, needed to reply to and resolve the thread. */ + root: ResourceComment; + } + | { + kind: "pr-review"; + prUrl: string; + filePath: string; + /** GitHub replies target a comment id, resolution a thread node id. */ + rootCommentId: number; + threadNodeId: string; + } + | { kind: "pr-conversation"; prUrl: string; url: string | null }; + +/** + * One thread in the task's comment list, whichever system it came from. The + * list renders and sorts these; only replying, resolving and opening still care + * where a thread lives, which is what `origin` carries. + */ +export type TaskCommentThread = { + /** Stable across refetches: the scroll target and React key. */ + id: string; + /** Groups threads for the source filter. */ + sourceKey: string; + sourceLabel: string; + sourceKind: "file" | "canvas" | "task" | "pr"; + entries: CommentEntry[]; + resolved: boolean; + /** Newest comment in the thread, for ordering the list. */ + lastActivityAt: string; + origin: ThreadOrigin; +}; + +function resourceAuthorName(comment: ResourceComment): string { + const user = comment.created_by; + if (!user) return "You"; + return ( + [user.first_name, user.last_name].filter(Boolean).join(" ") || user.email + ); +} + +function resourceEntry(comment: ResourceComment): CommentEntry { + return { + id: comment.id, + authorName: resourceAuthorName(comment), + user: comment.created_by, + avatarUrl: null, + createdAt: comment.created_at, + body: comment.content ?? "", + format: "mentions", + }; +} + +/** The task's own comment threads, tagged with the resource they belong to. */ +export function resourceCommentThreads( + comments: ResourceComment[], + sources: CommentSource[], +): TaskCommentThread[] { + const byItemId = new Map(); + for (const source of sources) byItemId.set(source.target.itemId, source); + + return buildCommentThreads(comments).flatMap((thread) => { + const source = thread.root.item_id + ? byItemId.get(thread.root.item_id) + : undefined; + if (!source) return []; + // A resolve/reopen reply is thread state, not something anyone said. + const visibleReplies = thread.replies.filter( + (reply) => !readCommentContext(reply)?.threadState, + ); + return [ + { + id: thread.root.id, + sourceKey: commentTargetKey(source.target), + sourceLabel: source.name, + sourceKind: source.kind, + entries: [thread.root, ...visibleReplies].map(resourceEntry), + resolved: thread.resolved, + lastActivityAt: + thread.replies.at(-1)?.created_at ?? thread.root.created_at, + origin: { kind: "resource", source, root: thread.root }, + }, + ]; + }); +} + +/** + * A PR's comments as threads. Inline review threads keep their replies and can + * be resolved; conversation comments (issue chatter, review summaries) are each + * a thread of one, since GitHub gives them neither replies nor resolution. + */ +export function prCommentThreads( + prUrl: string, + prLabel: string, + reviewThreads: PrReviewThread[], + conversation: PrConversationComment[], +): TaskCommentThread[] { + const threads: TaskCommentThread[] = reviewThreads.flatMap((thread) => { + const root = thread.comments[0]; + if (!root || root.user.isBot) return []; + const humanComments = thread.comments.filter( + (comment) => !comment.user.isBot, + ); + return [ + { + id: `pr-review-${thread.rootId}`, + sourceKey: prUrl, + sourceLabel: prLabel, + sourceKind: "pr" as const, + entries: humanComments.map((comment) => ({ + id: `pr-comment-${comment.id}`, + authorName: comment.user.login, + user: null, + avatarUrl: comment.user.avatar_url || null, + createdAt: comment.created_at, + body: comment.body, + format: "markdown" as const, + })), + resolved: thread.isResolved, + lastActivityAt: humanComments.at(-1)?.created_at ?? root.created_at, + origin: { + kind: "pr-review" as const, + prUrl, + filePath: thread.filePath, + rootCommentId: thread.rootId, + threadNodeId: thread.nodeId, + }, + }, + ]; + }); + + for (const comment of conversation) { + if (comment.isBot) continue; + threads.push({ + // Conversation items mix issue comments and review summaries, whose ids + // come from different GitHub id spaces — key on the timestamp too. + id: `pr-conversation-${comment.id}-${comment.createdAt}`, + sourceKey: prUrl, + sourceLabel: prLabel, + sourceKind: "pr", + entries: [ + { + id: `pr-conversation-${comment.id}`, + authorName: comment.author, + user: null, + avatarUrl: comment.avatarUrl, + createdAt: comment.createdAt, + body: comment.body, + format: "markdown", + }, + ], + resolved: false, + lastActivityAt: comment.createdAt, + origin: { kind: "pr-conversation", prUrl, url: comment.url }, + }); + } + + return threads; +} + +export function byNewestActivity( + a: TaskCommentThread, + b: TaskCommentThread, +): number { + return b.lastActivityAt.localeCompare(a.lastActivityAt); +} + +export type SourceKind = TaskCommentThread["sourceKind"]; +export type ThreadSourceOption = { + key: string; + label: string; + kind: SourceKind; + count: number; +}; + +/** + * The sources present in a set of threads, for the source filter. Newest-first + * like the list, except the task itself sits at the top (just under "All + * sources") since it's the one source every task has. + */ +export function threadSourceOptions( + threads: TaskCommentThread[], +): ThreadSourceOption[] { + const byKey = new Map(); + for (const thread of threads) { + const source = byKey.get(thread.sourceKey); + if (!source) { + byKey.set(thread.sourceKey, { + key: thread.sourceKey, + label: thread.sourceLabel, + kind: thread.sourceKind, + count: 1, + }); + } else { + source.count += 1; + } + } + return [...byKey.values()].sort( + (a, b) => Number(b.kind === "task") - Number(a.kind === "task"), + ); +} diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.test.tsx b/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.test.tsx index efbadfa48e49..c9586cade231 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.test.tsx @@ -122,4 +122,38 @@ describe("BuiltCanvas", () => { }); expect(onDataRequest).not.toHaveBeenCalled(); }); + + it("opens a comment selected inside the built artifact", async () => { + const onCommentActivate = vi.fn(); + render( + , + ); + const iframe = screen.getByTitle("Canvas") as HTMLIFrameElement; + if (!iframe.contentWindow) throw new Error("Canvas iframe has no window"); + const postMessage = vi + .spyOn(iframe.contentWindow, "postMessage") + .mockImplementation(() => undefined); + + fireEvent.load(iframe); + const calls = postMessage.mock.calls as unknown as [ + unknown, + string, + Transferable[], + ][]; + const canvasPort = calls.at(-1)?.[2]?.[0] as MessagePort; + canvasPort.postMessage({ + channel: "posthog-canvas", + type: "comment-activate", + id: "comment-1", + }); + + await waitFor(() => + expect(onCommentActivate).toHaveBeenCalledWith("comment-1"), + ); + }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.tsx b/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.tsx index 865393d7c5b9..d694a643bf1a 100644 --- a/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.tsx +++ b/products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.tsx @@ -1,12 +1,14 @@ import { assertCanvasCapability } from "@posthog/core/canvas/canvasCapabilities"; import { + type CanvasCommentHighlight, type CanvasNavIntent, + type CanvasTextSelection, canvasToHostMessageSchema, } from "@posthog/core/canvas/freeformSchemas"; import type { CanvasCapabilities } from "@posthog/shared"; import { logger } from "@posthog/ui/shell/logger"; import { openExternalUrl } from "@posthog/ui/shell/openExternal"; -import { useLayoutEffect, useRef } from "react"; +import { useEffect, useLayoutEffect, useRef } from "react"; import { createCanvasHostMessageRouter } from "./canvasHostMessageRouter"; const log = logger.scope("built-canvas"); @@ -81,6 +83,9 @@ export interface BuiltCanvasProps { onReady?: () => void; onRendered?: () => void; onNavigate?: (intent: CanvasNavIntent) => void; + onTextSelection?: (selection: CanvasTextSelection | null) => void; + onCommentActivate?: (id: string) => void; + commentHighlights?: CanvasCommentHighlight[]; } export function BuiltCanvas({ @@ -91,8 +96,12 @@ export function BuiltCanvas({ onReady, onRendered, onNavigate, + onTextSelection, + onCommentActivate, + commentHighlights = [], }: BuiltCanvasProps) { const iframeRef = useRef(null); + const artifactPortRef = useRef(null); const hostDocument = buildArtifactHostDocument(artifactUrl); const latest = useRef({ capabilities, @@ -101,6 +110,9 @@ export function BuiltCanvas({ onReady, onRendered, onNavigate, + onTextSelection, + onCommentActivate, + commentHighlights, }); latest.current = { capabilities, @@ -109,6 +121,9 @@ export function BuiltCanvas({ onReady, onRendered, onNavigate, + onTextSelection, + onCommentActivate, + commentHighlights, }; // biome-ignore lint/correctness/useExhaustiveDependencies: a new host document needs a fresh bridge even though the effect reads it only through the iframe. @@ -129,9 +144,33 @@ export function BuiltCanvas({ log.warn("Built canvas error", { message }); latest.current.onError?.(message, stack); }, - onReady: () => latest.current.onReady?.(), + onReady: () => { + artifactPort?.postMessage({ + channel: "posthog-canvas", + type: "set-comment-highlights", + highlights: latest.current.commentHighlights, + }); + latest.current.onReady?.(); + }, onRendered: () => latest.current.onRendered?.(), onNavigate: (intent) => latest.current.onNavigate?.(intent), + onTextSelection: (selection) => { + if (!selection) { + latest.current.onTextSelection?.(null); + return; + } + const frame = iframeRef.current?.getBoundingClientRect(); + latest.current.onTextSelection?.({ + ...selection, + rect: { + top: selection.rect.top + (frame?.top ?? 0), + right: selection.rect.right + (frame?.left ?? 0), + bottom: selection.rect.bottom + (frame?.top ?? 0), + left: selection.rect.left + (frame?.left ?? 0), + }, + }); + }, + onCommentActivate: (id) => latest.current.onCommentActivate?.(id), }), hasUserActivation: () => navigator.userActivation?.isActive === true, // Built artifacts run arbitrary published code, so an external open asks @@ -152,6 +191,7 @@ export function BuiltCanvas({ if (artifactPort) return; const bridge = new MessageChannel(); artifactPort = bridge.port1; + artifactPortRef.current = artifactPort; artifactPort.addEventListener("message", onMessage); artifactPort.start(); iframe?.contentWindow?.postMessage( @@ -171,6 +211,7 @@ export function BuiltCanvas({ } artifactPort?.close(); artifactPort = null; + artifactPortRef.current = null; }; iframe?.addEventListener("load", onLoad); @@ -179,9 +220,18 @@ export function BuiltCanvas({ iframe?.removeEventListener("load", onLoad); window.removeEventListener("message", onHostMessage); artifactPort?.close(); + artifactPortRef.current = null; }; }, [hostDocument]); + useEffect(() => { + artifactPortRef.current?.postMessage({ + channel: "posthog-canvas", + type: "set-comment-highlights", + highlights: commentHighlights, + }); + }, [commentHighlights]); + return (