diff --git a/products/posthog_ai/frontend/scenes/TaskTracker/components/TaskRunArtifacts.tsx b/products/posthog_ai/frontend/scenes/TaskTracker/components/TaskRunArtifacts.tsx index e314f79e8a79..161494916176 100644 --- a/products/posthog_ai/frontend/scenes/TaskTracker/components/TaskRunArtifacts.tsx +++ b/products/posthog_ai/frontend/scenes/TaskTracker/components/TaskRunArtifacts.tsx @@ -757,7 +757,7 @@ function ArtifactToolbar({ objectRef && currentProjectId !== null ? objectKindLink(objectRef.objectKind, objectRef.objectId, `/project/${currentProjectId}`) : null - const downloadUrl = artifactDownloadUrl(currentProjectId, taskId, artifact) + const downloadUrl = artifactDownloadUrl(currentProjectId, taskId, artifact, { forDownload: true }) const single = files.length < 2 const versioned = !!selectedFile && selectedFile.versions.length > 1 // Plain text already shows its source, so only these kinds get a view switch. diff --git a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.test.ts b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.test.ts index 5ea5eb87142c..be10084eea3e 100644 --- a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.test.ts +++ b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.test.ts @@ -254,7 +254,8 @@ describe('taskRunArtifacts', () => { function slackFile( location: Record, name = 'signups.png', - contentType = 'image/png' + contentType = 'image/png', + size = 2048 ): TaskRunLivingArtifactResponseApi { return livingArtifact({ id: 'doc-2', @@ -265,7 +266,7 @@ describe('taskRunArtifacts', () => { { version: 1, run_id: 'run-1', - size: 2048, + size, content_type: contentType, location, created_at: '2026-09-30T16:00:00Z', @@ -325,8 +326,16 @@ describe('taskRunArtifacts', () => { 'none', ], ['a Slack file with no stored copy has no preview', {}, 'image.png', 'image/png', 'none'], - ])('%s', (_, location, name, contentType, expected) => { - const [file] = livingArtifactFiles([slackFile(location, name, contentType)]) + [ + 'a stored Slack file video over the preview limit only downloads', + { storage_path: 'tasks/doc.v1.mp4' }, + 'demo.mp4', + 'video/mp4', + 'none', + 30 * 1024 * 1024, + ], + ])('%s', (_, location, name, contentType, expected, size = 2048) => { + const [file] = livingArtifactFiles([slackFile(location, name, contentType, size)]) expect(artifactPreviewKind(file.latest)).toBe(expected) }) }) diff --git a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.ts b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.ts index 046255ac4c52..075cdfb71ce3 100644 --- a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.ts +++ b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifacts.ts @@ -65,12 +65,19 @@ export function postHogObjectRef(artifact: TaskRunArtifactResponseApi): PostHogO return { objectKind: metadata.object_kind, objectId: metadata.object_id } } +// The app streams a living version preview through a web worker, so a larger file only downloads. +// Keep in step with LIVING_VERSION_PREVIEW_MAX_BYTES in the tasks backend. +export const LIVING_PREVIEW_MAX_BYTES = 25 * 1024 * 1024 + export function artifactPreviewKind( artifact: TaskRunArtifactResponseApi & { living?: LivingVersion } ): ArtifactPreviewKind { if (artifact.living && artifact.living.text === null) { + if (!artifact.living.stored || (artifact.size ?? 0) > LIVING_PREVIEW_MAX_BYTES) { + return 'none' + } // A stored file plays in an `img` or a `video` from its URL. Text needs a read of the body, so it downloads. - const kind = artifact.living.stored ? fileKind(artifact) : 'none' + const kind = fileKind(artifact) return kind === 'image' || kind === 'video' ? kind : 'none' } if (artifact.type === 'reference') { diff --git a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifactsLogic.ts b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifactsLogic.ts index 4cbf4bb4d32d..5256c13252f7 100644 --- a/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifactsLogic.ts +++ b/products/posthog_ai/frontend/scenes/TaskTracker/taskRunArtifactsLogic.ts @@ -370,7 +370,17 @@ const MAX_MEDIA_PREVIEW_BYTES = 200 * 1024 * 1024 * A URL that an `img`, a `video` or an `a` can use directly. The download-by-id URL redirects to a fresh * presigned link. A stored living version streams from the app origin. Other living versions have no URL. */ -export function artifactDownloadUrl(projectId: number | null, taskId: string, artifact: RunArtifact): string | null { +/** + * The URL of an artifact's file. With `forDownload`, a stored living version redirects to object storage, so a + * large file does not pass through the app. A preview keeps the app URL, because the media-src policy allows + * video only from the app origin. + */ +export function artifactDownloadUrl( + projectId: number | null, + taskId: string, + artifact: RunArtifact, + { forDownload = false }: { forDownload?: boolean } = {} +): string | null { if (projectId === null || !artifact.id) { return null } @@ -381,7 +391,8 @@ export function artifactDownloadUrl(projectId: number | null, taskId: string, ar taskId, artifact.runId, artifact.living.artifactId, - artifact.living.version + artifact.living.version, + forDownload ? { download: true } : undefined ) : null } diff --git a/products/tasks/backend/facade/api.py b/products/tasks/backend/facade/api.py index 583a0fa1be0a..308eafca5ff8 100644 --- a/products/tasks/backend/facade/api.py +++ b/products/tasks/backend/facade/api.py @@ -8,6 +8,7 @@ from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field, replace from datetime import UTC, datetime, timedelta +from pathlib import PurePosixPath from typing import Any, Literal, TypeVar from urllib.parse import urlparse from uuid import UUID, uuid4 @@ -323,6 +324,7 @@ class _AutoArchiveUnchanged: "prepare_task_staged_artifacts", "presign_task_run_artifact", "presign_task_run_artifact_download", + "presign_task_run_living_artifact_version_download", "read_task_run_artifact", "read_task_run_living_artifact_version", "get_task_run_log_urls", @@ -4375,10 +4377,11 @@ def read_task_run_living_artifact_version( """Read the content of one living artifact version. Returns ``(content, error)``: ``(None, None)`` if the run isn't found, ``(None, "not_found")`` if the - artifact or version isn't found or keeps no content, ``(None, "read_failed")`` if the storage read - raised, else ``(content, None)``. + artifact or version isn't found or keeps no content, ``(None, "too_large")`` if a stored version is + over the preview limit, ``(None, "read_failed")`` if the storage read raised, else ``(content, None)``. """ from products.tasks.backend.logic.services.living_artifacts import ( # noqa: PLC0415 — keep storage deps off the api import path + LivingArtifactVersionTooLarge, get_task_artifact_for_run, read_living_artifact_version, ) @@ -4395,6 +4398,8 @@ def read_task_run_living_artifact_version( return None, "not_found" try: content = read_living_artifact_version(artifact, version) + except LivingArtifactVersionTooLarge: + return None, "too_large" except Exception: logger.exception("Failed to read living artifact %s version %s for team %s", artifact_id, version, team_id) return None, "read_failed" @@ -4403,6 +4408,49 @@ def read_task_run_living_artifact_version( return content, None +def presign_task_run_living_artifact_version_download( + run_id: str | UUID, task_id: str | UUID, team_id: int, *, artifact_id: str | UUID, version: int +) -> contracts.LivingArtifactVersionDownload: + """Presign a download URL for one stored living artifact version. + + The error is ``"not_found"`` if the artifact or version isn't found, ``"not_stored"`` if the version + keeps its content as text, and ``"unavailable"`` if presigning fails. Both fields are None if the run + isn't found. + """ + from posthog.storage import object_storage # noqa: PLC0415 — keep storage deps off the api import path + + from products.tasks.backend.logic.services.living_artifacts import ( # noqa: PLC0415 — keep storage deps off the api import path + get_task_artifact_for_run, + resolve_living_artifact_version, + ) + + run = _get_visible_run(run_id, task_id, team_id) + if run is None: + return contracts.LivingArtifactVersionDownload(url=None, error=None) + try: + UUID(str(artifact_id)) + except ValueError: + return contracts.LivingArtifactVersionDownload(url=None, error="not_found") + artifact = get_task_artifact_for_run(run, artifact_id) + resolved = resolve_living_artifact_version(artifact, version) if artifact is not None else None + if artifact is None or resolved is None: + return contracts.LivingArtifactVersionDownload(url=None, error="not_found") + if not resolved.storage_path: + return contracts.LivingArtifactVersionDownload(url=None, error="not_stored") + url = object_storage.get_presigned_url( + resolved.storage_path, + content_type=resolved.content_type or None, + # Agent-written HTML or SVG must not render as a page, so the browser always saves it. + content_disposition=content_disposition_header( + as_attachment=True, filename=PurePosixPath(artifact.name).name or "artifact" + ) + or "attachment", + ) + if not url: + return contracts.LivingArtifactVersionDownload(url=None, error="unavailable") + return contracts.LivingArtifactVersionDownload(url=url, error=None) + + def create_task_run_living_artifact( run_id: str | UUID, task_id: str | UUID, diff --git a/products/tasks/backend/facade/contracts.py b/products/tasks/backend/facade/contracts.py index d2eb97f9c108..1d313ac3e683 100644 --- a/products/tasks/backend/facade/contracts.py +++ b/products/tasks/backend/facade/contracts.py @@ -1028,3 +1028,9 @@ class LivingArtifactVersionContent: name: str content_type: str content: bytes + + +@dataclass(frozen=True, kw_only=True) +class LivingArtifactVersionDownload: + url: str | None + error: Literal["not_found", "not_stored", "unavailable"] | None diff --git a/products/tasks/backend/logic/services/living_artifacts.py b/products/tasks/backend/logic/services/living_artifacts.py index 4e98221fbfc7..e904ea352d12 100644 --- a/products/tasks/backend/logic/services/living_artifacts.py +++ b/products/tasks/backend/logic/services/living_artifacts.py @@ -24,6 +24,7 @@ import structlog from slack_sdk.errors import SlackApiError +from posthog.dataclasses import frozen from posthog.event_usage import groups from posthog.ph_client import ph_scoped_capture from posthog.slack.channels import MAX_BUTTON_URL_CHARS, SlackButton, section_block @@ -377,12 +378,25 @@ def open_task_artifact(artifact: TaskArtifact) -> str | None: return _adapter_for_existing_artifact(artifact).open(artifact) -def read_living_artifact_version(artifact: TaskArtifact, version: int) -> LivingArtifactVersionContent | None: - """Return the content of one version, or None when the version is unknown or keeps no content. +# The app streams a preview through a web worker, so a larger stored version only downloads. +# Keep in step with LIVING_PREVIEW_MAX_BYTES in the TaskTracker frontend. +LIVING_VERSION_PREVIEW_MAX_BYTES = 25 * 1024 * 1024 - A Slack file version keeps its bytes in object storage. A canvas or message version keeps its - text in the version record. Storage read errors propagate to the caller. - """ + +class LivingArtifactVersionTooLarge(Exception): + pass + + +@frozen +class LivingVersionLocation: + record: dict[str, Any] + content_type: str + # Empty when the version keeps its content as text in the record. + storage_path: str + + +def resolve_living_artifact_version(artifact: TaskArtifact, version: int) -> LivingVersionLocation | None: + """Find one version and where it keeps its content, or None when the version is unknown or its path is foreign.""" record = next( ( candidate @@ -398,20 +412,47 @@ def read_living_artifact_version(artifact: TaskArtifact, version: int) -> Living content_type = str(record.get("content_type") or location.get("content_type") or "") or _guess_content_type( artifact.name ) - storage_path = str(location.get("storage_path") or "") - if storage_path: - # Every living artifact object sits under its task's prefix. A path outside it is not this artifact's object. - if not storage_path.startswith(_task_artifact_s3_prefix(artifact)): - return None - payload = object_storage.read_bytes(storage_path, missing_ok=True) + # Every living artifact object sits under its task's prefix. A path outside it is not this artifact's object. + if storage_path and not storage_path.startswith(_task_artifact_s3_prefix(artifact)): + return None + return LivingVersionLocation(record=record, content_type=content_type, storage_path=storage_path) + + +def _stored_version_size(resolved: LivingVersionLocation) -> int | None: + size = resolved.record.get("size") + if isinstance(size, int): + return size + head = object_storage.head_object(resolved.storage_path) + length = head.get("ContentLength") if head else None + return length if isinstance(length, int) else None + + +def read_living_artifact_version(artifact: TaskArtifact, version: int) -> LivingArtifactVersionContent | None: + """Return the content of one version, or None when the version is unknown or keeps no content. + + A Slack file version keeps its bytes in object storage. A canvas or message version keeps its + text in the version record. A stored version above the preview limit raises + LivingArtifactVersionTooLarge. Storage read errors propagate to the caller. + """ + resolved = resolve_living_artifact_version(artifact, version) + if resolved is None: + return None + + if resolved.storage_path: + size = _stored_version_size(resolved) + if size is not None and size > LIVING_VERSION_PREVIEW_MAX_BYTES: + raise LivingArtifactVersionTooLarge() + payload = object_storage.read_bytes(resolved.storage_path, missing_ok=True) if payload is None: return None - return LivingArtifactVersionContent(name=artifact.name, content_type=content_type, content=payload) + return LivingArtifactVersionContent(name=artifact.name, content_type=resolved.content_type, content=payload) - text = record.get("content") + text = resolved.record.get("content") if isinstance(text, str): - return LivingArtifactVersionContent(name=artifact.name, content_type=content_type, content=text.encode("utf-8")) + return LivingArtifactVersionContent( + name=artifact.name, content_type=resolved.content_type, content=text.encode("utf-8") + ) return None diff --git a/products/tasks/backend/presentation/views/api.py b/products/tasks/backend/presentation/views/api.py index d2105e81e58f..200c06c2d327 100644 --- a/products/tasks/backend/presentation/views/api.py +++ b/products/tasks/backend/presentation/views/api.py @@ -4519,20 +4519,36 @@ def edit(self, request, pk=None, **kwargs): OpenApiTypes.INT, OpenApiParameter.PATH, description="Version number of the living artifact, as listed in its versions.", - ) + ), + OpenApiParameter( + "download", + OpenApiTypes.BOOL, + OpenApiParameter.QUERY, + required=False, + description=( + "Set to true to save the version. A stored file then redirects to a short-lived presigned " + "URL, so a large file never passes through the app. Leave unset for an inline preview." + ), + ), ], responses={ (200, "application/octet-stream"): OpenApiResponse( response=OpenApiTypes.BINARY, description="Version content, with the content type the version was saved with", ), + 302: OpenApiResponse(description="With download=true, a redirect to a presigned URL for the stored file"), 400: OpenApiResponse(response=TaskRunErrorResponseSerializer, description="Unable to read the version"), 404: OpenApiResponse(description="Living artifact or version not found, or the version keeps no content"), + 413: OpenApiResponse( + response=TaskRunErrorResponseSerializer, + description="The stored file is too large to preview. Request it with download=true.", + ), }, summary="Download one version of a living artifact", description=( - "Streams the content of one living artifact version from the app origin. Slack file versions return " - "their stored file. Slack canvas and message versions return their text." + "Returns the content of one living artifact version. Slack file versions return their stored file, " + "streamed from the app origin for a preview or redirected to a presigned URL with download=true. " + "Slack canvas and message versions return their text." ), operation_id="tasks_runs_living_artifacts_version_content", ) @@ -4544,9 +4560,32 @@ def edit(self, request, pk=None, **kwargs): ) def version_content(self, request, pk=None, version=None, **kwargs): task_id = self._ensure_task_accessible() + version_number = int(str(version)) + if str(request.query_params.get("download", "")).lower() in ("1", "true"): + download = tasks_facade.presign_task_run_living_artifact_version_download( + self._run_id(), task_id, self.team_id, artifact_id=str(pk), version=version_number + ) + if download.url: + redirect = HttpResponseRedirect(download.url) + redirect["Cache-Control"] = "no-store" + return redirect + if download.error == "unavailable": + return Response( + TaskRunErrorResponseSerializer({"error": "Unable to read this version"}).data, + status=status.HTTP_400_BAD_REQUEST, + ) + if download.error != "not_stored": + raise NotFound() content, error = tasks_facade.read_task_run_living_artifact_version( - self._run_id(), task_id, self.team_id, artifact_id=str(pk), version=int(str(version)) + self._run_id(), task_id, self.team_id, artifact_id=str(pk), version=version_number ) + if error == "too_large": + return Response( + TaskRunErrorResponseSerializer( + {"error": "This file is too large to preview. Download it instead."} + ).data, + status=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + ) if error == "read_failed": return Response( TaskRunErrorResponseSerializer({"error": "Unable to read this version"}).data, diff --git a/products/tasks/backend/tests/test_api.py b/products/tasks/backend/tests/test_api.py index 4be6816b4fd1..e2c5a0dd48bd 100644 --- a/products/tasks/backend/tests/test_api.py +++ b/products/tasks/backend/tests/test_api.py @@ -8580,7 +8580,9 @@ def test_living_artifact_create_requires_content_or_source(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def _create_slack_file_living_artifact(self, task: Task, run: TaskRun, storage_path: str | None = None): + def _create_slack_file_living_artifact( + self, task: Task, run: TaskRun, storage_path: str | None = None, size: int | None = None + ): path = storage_path or f"{run.get_artifact_s3_prefix()}/living/doc/signups.v1.png" return TaskArtifact.objects.for_team(task.team_id).create( team_id=task.team_id, @@ -8597,6 +8599,7 @@ def _create_slack_file_living_artifact(self, task: Task, run: TaskRun, storage_p "run_id": str(run.id), "content_type": "image/png", "location": {"kind": "slack_file", "storage_path": path}, + **({"size": size} if size is not None else {}), }, { "version": 2, @@ -8631,6 +8634,36 @@ def test_living_artifact_version_content(self, _name, version, expected_body, ex self.assertEqual(response["Content-Type"], expected_type) self.assertEqual(response["Content-Disposition"], 'attachment; filename="signups.png"') + @parameterized.expand( + [ + ("download_stored_file_redirects", 1, "?download=true", None, 302), + ("download_text_streams", 2, "?download=true", None, 200), + ("preview_over_limit_is_refused", 1, "", 26 * 1024 * 1024, 413), + ("download_over_limit_redirects", 1, "?download=1", 26 * 1024 * 1024, 302), + ] + ) + @patch("posthog.storage.object_storage.get_presigned_url") + @patch("posthog.storage.object_storage.read_bytes") + def test_living_artifact_version_download( + self, _name, version, query, size, expected_status, mock_read_bytes, mock_presign + ): + mock_presign.return_value = "https://storage.example.com/signups.v1.png?signature=fake" + task = self.create_task() + run = TaskRun.objects.create(task=task, team=self.team, status=TaskRun.Status.IN_PROGRESS) + artifact = self._create_slack_file_living_artifact(task, run, size=size) + + response = self.client.get( + f"/api/projects/@current/tasks/{task.id}/runs/{run.id}/living_artifacts/{artifact.id}/versions/{version}/{query}" + ) + + self.assertEqual(response.status_code, expected_status) + if expected_status == 302: + self.assertEqual(response["Location"], mock_presign.return_value) + self.assertEqual(mock_presign.call_args.kwargs["content_disposition"], 'attachment; filename="signups.png"') + if expected_status == 200: + self.assertEqual(response.content, b"# Weekly signups") + mock_read_bytes.assert_not_called() + @parameterized.expand( [ ("unknown_version",), diff --git a/products/tasks/frontend/generated/api.schemas.ts b/products/tasks/frontend/generated/api.schemas.ts index d7169472636e..6a5b3b6a1f70 100644 --- a/products/tasks/frontend/generated/api.schemas.ts +++ b/products/tasks/frontend/generated/api.schemas.ts @@ -6160,6 +6160,13 @@ export type TasksRunsStreamTokenRetrieveParams = { resync?: boolean } +export type TasksRunsLivingArtifactsVersionContentParams = { + /** + * Set to true to save the version. A stored file then redirects to a short-lived presigned URL, so a large file never passes through the app. Leave unset for an inline preview. + */ + download?: boolean +} + export type TasksThreadMessagesListParams = { /** * Number of results to return per page. diff --git a/products/tasks/frontend/generated/api.ts b/products/tasks/frontend/generated/api.ts index 541ad496a658..7d932c55f8b6 100644 --- a/products/tasks/frontend/generated/api.ts +++ b/products/tasks/frontend/generated/api.ts @@ -157,6 +157,7 @@ import type { TasksRepositoryReadinessRetrieveParams, TasksReviewRetrieveParams, TasksRunsListParams, + TasksRunsLivingArtifactsVersionContentParams, TasksRunsSessionLogsRetrieveParams, TasksRunsStreamRetrieveParams, TasksRunsStreamTokenRetrieveParams, @@ -2682,13 +2683,26 @@ export const getTasksRunsLivingArtifactsVersionContentUrl = ( taskId: string, runId: string, id: string, - version: number + version: number, + params?: TasksRunsLivingArtifactsVersionContentParams ) => { - return `/api/projects/${projectId}/tasks/${taskId}/runs/${runId}/living_artifacts/${id}/versions/${version}/` + const normalizedParams = new URLSearchParams() + + Object.entries(params || {}).forEach(([key, value]) => { + if (value !== undefined) { + normalizedParams.append(key, value === null ? 'null' : String(value)) + } + }) + + const stringifiedParams = normalizedParams.toString() + + return stringifiedParams.length > 0 + ? `/api/projects/${projectId}/tasks/${taskId}/runs/${runId}/living_artifacts/${id}/versions/${version}/?${stringifiedParams}` + : `/api/projects/${projectId}/tasks/${taskId}/runs/${runId}/living_artifacts/${id}/versions/${version}/` } /** - * Streams the content of one living artifact version from the app origin. Slack file versions return their stored file. Slack canvas and message versions return their text. + * Returns the content of one living artifact version. Slack file versions return their stored file, streamed from the app origin for a preview or redirected to a presigned URL with download=true. Slack canvas and message versions return their text. * @summary Download one version of a living artifact */ export const tasksRunsLivingArtifactsVersionContent = async ( @@ -2697,12 +2711,16 @@ export const tasksRunsLivingArtifactsVersionContent = async ( runId: string, id: string, version: number, + params?: TasksRunsLivingArtifactsVersionContentParams, options?: RequestInit ): Promise => { - return apiMutator(getTasksRunsLivingArtifactsVersionContentUrl(projectId, taskId, runId, id, version), { - ...options, - method: 'GET', - }) + return apiMutator( + getTasksRunsLivingArtifactsVersionContentUrl(projectId, taskId, runId, id, version, params), + { + ...options, + method: 'GET', + } + ) } export const getTasksRunsLivingArtifactsChartUrl = (projectId: string, taskId: string, runId: string) => { diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index c6d9cd79d106..d870e1ddb358 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -122698,6 +122698,13 @@ export namespace Schemas { resync?: boolean; }; + export type TasksRunsLivingArtifactsVersionContentParams = { + /** + * Set to true to save the version. A stored file then redirects to a short-lived presigned URL, so a large file never passes through the app. Leave unset for an inline preview. + */ + download?: boolean; + }; + export type TasksThreadMessagesListParams = { /** * Number of results to return per page.