diff --git a/products/tasks/backend/facade/api.py b/products/tasks/backend/facade/api.py index 9c104beee95f..93273c37fddb 100644 --- a/products/tasks/backend/facade/api.py +++ b/products/tasks/backend/facade/api.py @@ -2795,6 +2795,7 @@ def finalize_task_run_artifact_uploads( manifest = list(run.artifacts or []) artifact_prefix = f"{run.get_artifact_s3_prefix()}/" finalized_entries: list[dict] = [] + new_entries: list[dict] = [] new_storage_paths: list[str] = [] for artifact in artifacts: @@ -2835,10 +2836,21 @@ def finalize_task_run_artifact_uploads( metadata=artifact.get("metadata"), ) manifest.append(entry) + new_entries.append(entry) finalized_entries.append(entry) new_storage_paths.append(storage_path) - _save_artifact_manifest(run, manifest) + if new_entries: + # Re-read the manifest under the row lock rather than writing back the snapshot taken + # above: verifying the uploads does S3 I/O, and a dismissal that commits in that window + # would be silently reverted by a blind whole-array write. + with transaction.atomic(): + locked_run = TaskRun.objects.select_for_update().get(pk=run.pk) + new_ids = {entry["id"] for entry in new_entries} + merged = [entry for entry in (locked_run.artifacts or []) if entry.get("id") not in new_ids] + merged.extend(new_entries) + _save_artifact_manifest(locked_run, merged) + for storage_path in new_storage_paths: _tag_artifact_object(run, storage_path) @@ -2976,6 +2988,48 @@ def presign_task_run_artifact( return url, None +def _without_dismissal(entry: dict) -> dict: + return {key: value for key, value in entry.items() if key != "dismissed_at"} + + +def set_task_run_artifacts_dismissed( + run_id: str | UUID, task_id: str | UUID, team_id: int, *, artifact_ids: list[str], dismissed: bool +) -> tuple[list[dict] | None, str | None]: + """Mark run artifacts as dismissed, or bring them back. + + Dismissal is a ``dismissed_at`` stamp on the manifest entry rather than a delete: the object + stays in storage until its TTL expires, so a file dismissed by mistake can be restored. + + Returns ``(manifest, error)``: ``(None, None)`` when the run isn't found, ``(None, "not_found")`` + when an id isn't on the run, else ``(updated_manifest, None)``. + """ + run = _get_visible_run(run_id, task_id, team_id) + if run is None: + return None, None + + with transaction.atomic(): + locked_run = TaskRun.objects.select_for_update().get(pk=run.pk) + manifest = list(locked_run.artifacts or []) + requested = set(artifact_ids) + if not requested.issubset({entry.get("id") for entry in manifest}): + return None, "not_found" + + # Restoring drops the key rather than nulling it, so a manifest entry only ever carries + # ``dismissed_at`` while it is dismissed and the response shape stays a plain optional. + dismissed_at = django_timezone.now().isoformat() + manifest = [ + ( + ({**entry, "dismissed_at": dismissed_at} if dismissed else _without_dismissal(entry)) + if entry.get("id") in requested + else entry + ) + for entry in manifest + ] + _save_artifact_manifest(locked_run, manifest) + + return manifest, None + + def read_task_run_artifact( run_id: str | UUID, task_id: str | UUID, team_id: int, *, storage_path: str ) -> tuple[bytes | None, dict | None, str | None]: diff --git a/products/tasks/backend/presentation/serializers.py b/products/tasks/backend/presentation/serializers.py index f62d7729864f..796479deef0e 100644 --- a/products/tasks/backend/presentation/serializers.py +++ b/products/tasks/backend/presentation/serializers.py @@ -280,6 +280,10 @@ class TaskRunArtifactResponseSerializer(serializers.Serializer): ) storage_path = serializers.CharField(help_text="S3 object key for the artifact") uploaded_at = serializers.CharField(help_text="Timestamp when the artifact was uploaded") + dismissed_at = serializers.CharField( + required=False, + help_text="Timestamp when a user dismissed the artifact. Absent while the artifact is shown.", + ) url = serializers.URLField( required=False, help_text=( @@ -1391,6 +1395,26 @@ class TaskRunArtifactPresignResponseSerializer(serializers.Serializer): expires_in = serializers.IntegerField(help_text="URL expiry in seconds") +class TaskRunArtifactsDismissRequestSerializer(serializers.Serializer): + artifact_ids = serializers.ListField( + child=serializers.CharField(max_length=128), + allow_empty=False, + max_length=100, + help_text=( + "Manifest ids of the artifacts to update. Pass every version of a file together so the " + "whole file is dismissed rather than a single upload of it." + ), + ) + dismissed = serializers.BooleanField( + default=True, + help_text="True to hide the artifacts from clients, false to show them again.", + ) + + +class TaskRunArtifactsDismissResponseSerializer(serializers.Serializer): + artifacts = TaskRunArtifactResponseSerializer(many=True, help_text="Updated list of artifacts on the run") + + TASK_SUMMARIES_MAX_IDS = 5000 diff --git a/products/tasks/backend/presentation/views/api.py b/products/tasks/backend/presentation/views/api.py index 293101f9fb16..ff6192a6ca03 100644 --- a/products/tasks/backend/presentation/views/api.py +++ b/products/tasks/backend/presentation/views/api.py @@ -110,6 +110,8 @@ TaskRunAppendLogRequestSerializer, TaskRunArtifactPresignRequestSerializer, TaskRunArtifactPresignResponseSerializer, + TaskRunArtifactsDismissRequestSerializer, + TaskRunArtifactsDismissResponseSerializer, TaskRunArtifactsFinalizeUploadRequestSerializer, TaskRunArtifactsFinalizeUploadResponseSerializer, TaskRunArtifactsPrepareUploadRequestSerializer, @@ -1663,6 +1665,46 @@ def artifacts_presign(self, request, pk=None, **kwargs): serializer = TaskRunArtifactPresignResponseSerializer({"url": url, "expires_in": 3600}) return Response(serializer.data) + @validated_request( + request_serializer=TaskRunArtifactsDismissRequestSerializer, + responses={ + 200: OpenApiResponse( + response=TaskRunArtifactsDismissResponseSerializer, + description="Run with updated artifact manifest", + ), + 404: OpenApiResponse(description="Artifact not found"), + }, + summary="Dismiss or restore task run artifacts", + description=( + "Hides artifacts from clients without deleting them from storage, so a file dismissed " + "by mistake can be restored." + ), + strict_request_validation=True, + ) + @action( + detail=True, + methods=["post"], + url_path="artifacts/dismiss", + required_scopes=["task:write"], + ) + def artifacts_dismiss(self, request, pk=None, **kwargs): + task_id = self._ensure_task_accessible() + manifest, error = tasks_facade.set_task_run_artifacts_dismissed( + pk, + task_id, + self.team_id, + artifact_ids=request.validated_data["artifact_ids"], + dismissed=request.validated_data["dismissed"], + ) + if error == "not_found": + return Response( + TaskRunErrorResponseSerializer({"error": "Artifact not found on this run"}).data, + status=status.HTTP_404_NOT_FOUND, + ) + if manifest is None: + raise NotFound() + return Response(TaskRunArtifactsDismissResponseSerializer({"artifacts": manifest}).data) + @validated_request( request_serializer=TaskRunArtifactPresignRequestSerializer, responses={ diff --git a/products/tasks/backend/tests/test_api.py b/products/tasks/backend/tests/test_api.py index 4b0f8dad89da..81dfb45aac82 100644 --- a/products/tasks/backend/tests/test_api.py +++ b/products/tasks/backend/tests/test_api.py @@ -7614,6 +7614,105 @@ def test_cancel_unknown_run_returns_404(self): self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) +class TestTaskRunArtifactDismissAPI(BaseTaskAPITest): + def _create_run_with_artifacts(self) -> tuple[Task, TaskRun]: + task = self.create_task() + run = task.create_run(environment=TaskRun.Environment.CLOUD) + run.artifacts = [ + build_task_artifact_entry( + artifact_id=artifact_id, + name=name, + artifact_type="output", + source="agent_output", + size=1024, + content_type="text/markdown", + storage_path=f"tasks/artifacts/team_{self.team.id}/task_{task.id}/run_{run.id}/{artifact_id}_{name}", + ) + for artifact_id, name in ( + ("artifact-1", "report.md"), + ("artifact-2", "report.md"), + ("artifact-3", "chart.png"), + ) + ] + run.save(update_fields=["artifacts", "updated_at"]) + return task, run + + def _dismiss(self, task: Task, run: TaskRun, artifact_ids: list[str], dismissed: bool): + return self.client.post( + f"/api/projects/@current/tasks/{task.id}/runs/{run.id}/artifacts/dismiss/", + {"artifact_ids": artifact_ids, "dismissed": dismissed}, + format="json", + ) + + def test_dismiss_flags_only_the_requested_artifacts_and_can_be_undone(self): + task, run = self._create_run_with_artifacts() + + response = self._dismiss(task, run, ["artifact-1", "artifact-2"], True) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + run.refresh_from_db() + dismissed_at = {artifact["id"]: artifact.get("dismissed_at") for artifact in run.artifacts} + self.assertIsNotNone(dismissed_at["artifact-1"]) + self.assertIsNotNone(dismissed_at["artifact-2"]) + self.assertIsNone(dismissed_at.get("artifact-3")) + self.assertEqual([artifact["id"] for artifact in response.json()["artifacts"]], list(dismissed_at)) + + self.assertEqual(self._dismiss(task, run, ["artifact-1", "artifact-2"], False).status_code, status.HTTP_200_OK) + run.refresh_from_db() + self.assertTrue(all("dismissed_at" not in artifact for artifact in run.artifacts)) + + def test_dismiss_unknown_artifact_leaves_the_manifest_alone(self): + task, run = self._create_run_with_artifacts() + + response = self._dismiss(task, run, ["artifact-1", "artifact-missing"], True) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json()["error"], "Artifact not found on this run") + run.refresh_from_db() + self.assertTrue(all(artifact.get("dismissed_at") is None for artifact in run.artifacts)) + + @patch("posthog.storage.object_storage.get_presigned_url") + @patch("posthog.storage.object_storage.tag") + @patch("posthog.storage.object_storage.head_object") + def test_dismissal_survives_a_finalize_upload_that_overlaps_it(self, mock_head_object, mock_tag, mock_presign): + task, run = self._create_run_with_artifacts() + new_artifact_id = uuid.uuid4().hex + storage_path = f"{run.get_artifact_s3_prefix()}/{new_artifact_id[:8]}_summary.pdf" + mock_presign.return_value = None + + # Dismissing from inside head_object lands the dismissal in the window where finalize has + # read the manifest but not yet written it — the overlap when an agent revises a file the + # user is dismissing. + def dismiss_mid_flight(_storage_path): + self._dismiss(task, run, ["artifact-1"], True) + return {"ContentLength": 4096, "ContentType": "application/pdf"} + + mock_head_object.side_effect = dismiss_mid_flight + + response = self.client.post( + f"/api/projects/@current/tasks/{task.id}/runs/{run.id}/artifacts/finalize_upload/", + { + "artifacts": [ + { + "id": new_artifact_id, + "name": "summary.pdf", + "type": "output", + "source": "agent_output", + "storage_path": storage_path, + "content_type": "application/pdf", + } + ] + }, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + run.refresh_from_db() + artifacts_by_id = {artifact["id"]: artifact for artifact in run.artifacts} + self.assertIsNotNone(artifacts_by_id["artifact-1"].get("dismissed_at")) + self.assertIn(new_artifact_id, artifacts_by_id) + + class TestTaskRunSessionLogsAPI(BaseTaskAPITest): """Tests for the GET .../session_logs/ endpoint that returns filtered log entries.""" diff --git a/products/tasks/frontend/generated/api.schemas.ts b/products/tasks/frontend/generated/api.schemas.ts index ffd772e72d08..a48fcdc2b3a9 100644 --- a/products/tasks/frontend/generated/api.schemas.ts +++ b/products/tasks/frontend/generated/api.schemas.ts @@ -1434,6 +1434,8 @@ export interface TaskRunArtifactResponseApi { storage_path: string /** Timestamp when the artifact was uploaded */ uploaded_at: string + /** Timestamp when a user dismissed the artifact. Absent while the artifact is shown. */ + dismissed_at?: string /** Presigned download URL for the artifact. Populated on the finalize-upload response so the caller can link to the file directly; it is time-limited and not persisted on the manifest. */ url?: string } @@ -2979,6 +2981,22 @@ export interface TaskRunArtifactsUploadResponseApi { artifacts: TaskRunArtifactResponseApi[] } +export interface TaskRunArtifactsDismissRequestApi { + /** + * Manifest ids of the artifacts to update. Pass every version of a file together so the whole file is dismissed rather than a single upload of it. + * @maxItems 100 + * @items.maxLength 128 + */ + artifact_ids: string[] + /** True to hide the artifacts from clients, false to show them again. */ + dismissed?: boolean +} + +export interface TaskRunArtifactsDismissResponseApi { + /** Updated list of artifacts on the run */ + artifacts: TaskRunArtifactResponseApi[] +} + export interface TaskRunArtifactPresignRequestApi { /** * S3 storage path returned in the artifact manifest diff --git a/products/tasks/frontend/generated/api.ts b/products/tasks/frontend/generated/api.ts index d2a45f71e586..60d9c09430e5 100644 --- a/products/tasks/frontend/generated/api.ts +++ b/products/tasks/frontend/generated/api.ts @@ -85,6 +85,8 @@ import type { TaskRunAppendLogRequestApi, TaskRunArtifactPresignRequestApi, TaskRunArtifactPresignResponseApi, + TaskRunArtifactsDismissRequestApi, + TaskRunArtifactsDismissResponseApi, TaskRunArtifactsFinalizeUploadRequestApi, TaskRunArtifactsFinalizeUploadResponseApi, TaskRunArtifactsPrepareUploadRequestApi, @@ -1687,6 +1689,32 @@ export const tasksRunsArtifactsCreate = async ( }) } +export const getTasksRunsArtifactsDismissCreateUrl = (projectId: string, taskId: string, id: string) => { + return `/api/projects/${projectId}/tasks/${taskId}/runs/${id}/artifacts/dismiss/` +} + +/** + * Hides artifacts from clients without deleting them from storage, so a file dismissed by mistake can be restored. + * @summary Dismiss or restore task run artifacts + */ +export const tasksRunsArtifactsDismissCreate = async ( + projectId: string, + taskId: string, + id: string, + taskRunArtifactsDismissRequestApi: TaskRunArtifactsDismissRequestApi, + options?: RequestInit +): Promise => { + return apiMutator( + getTasksRunsArtifactsDismissCreateUrl(projectId, taskId, id), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...options?.headers }, + body: JSON.stringify(taskRunArtifactsDismissRequestApi), + } + ) +} + export const getTasksRunsArtifactsDownloadCreateUrl = (projectId: string, taskId: string, id: string) => { return `/api/projects/${projectId}/tasks/${taskId}/runs/${id}/artifacts/download/` } diff --git a/products/tasks/frontend/generated/api.zod.ts b/products/tasks/frontend/generated/api.zod.ts index ad982e1e1ef7..fe0283470906 100644 --- a/products/tasks/frontend/generated/api.zod.ts +++ b/products/tasks/frontend/generated/api.zod.ts @@ -2497,6 +2497,29 @@ export const TasksRunsArtifactsCreateBody = /* @__PURE__ */ zod.object({ .describe('Array of artifacts to upload'), }) +/** + * Hides artifacts from clients without deleting them from storage, so a file dismissed by mistake can be restored. + * @summary Dismiss or restore task run artifacts + */ +export const tasksRunsArtifactsDismissCreateBodyArtifactIdsItemMax = 128 + +export const tasksRunsArtifactsDismissCreateBodyArtifactIdsMax = 100 + +export const tasksRunsArtifactsDismissCreateBodyDismissedDefault = true + +export const TasksRunsArtifactsDismissCreateBody = /* @__PURE__ */ zod.object({ + artifact_ids: zod + .array(zod.string().max(tasksRunsArtifactsDismissCreateBodyArtifactIdsItemMax)) + .max(tasksRunsArtifactsDismissCreateBodyArtifactIdsMax) + .describe( + 'Manifest ids of the artifacts to update. Pass every version of a file together so the whole file is dismissed rather than a single upload of it.' + ), + dismissed: zod + .boolean() + .default(tasksRunsArtifactsDismissCreateBodyDismissedDefault) + .describe('True to hide the artifacts from clients, false to show them again.'), +}) + /** * Streams artifact content for a task run artifact after validating that it belongs to the run. * @summary Download an artifact through the backend diff --git a/products/tasks/mcp/tools.yaml b/products/tasks/mcp/tools.yaml index 5149143afb02..9e2f3da1229a 100644 --- a/products/tasks/mcp/tools.yaml +++ b/products/tasks/mcp/tools.yaml @@ -444,6 +444,9 @@ tools: tasks-runs-artifacts-create: operation: tasks_runs_artifacts_create enabled: false + tasks-runs-artifacts-dismiss-create: + operation: tasks_runs_artifacts_dismiss_create + enabled: false tasks-runs-artifacts-download-create: operation: tasks_runs_artifacts_download_create enabled: false diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index 83fc61958715..1ba94f94c080 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -49564,6 +49564,8 @@ export namespace Schemas { storage_path: string; /** Timestamp when the artifact was uploaded */ uploaded_at: string; + /** Timestamp when a user dismissed the artifact. Absent while the artifact is shown. */ + dismissed_at?: string; /** Presigned download URL for the artifact. Populated on the finalize-upload response so the caller can link to the file directly; it is time-limited and not persisted on the manifest. */ url?: string; } @@ -73706,6 +73708,22 @@ export namespace Schemas { metadata?: TaskRunArtifactMetadata; } + export interface TaskRunArtifactsDismissRequest { + /** + * Manifest ids of the artifacts to update. Pass every version of a file together so the whole file is dismissed rather than a single upload of it. + * @maxItems 100 + * @items.maxLength 128 + */ + artifact_ids: string[]; + /** True to hide the artifacts from clients, false to show them again. */ + dismissed?: boolean; + } + + export interface TaskRunArtifactsDismissResponse { + /** Updated list of artifacts on the run */ + artifacts: TaskRunArtifactResponse[]; + } + export interface TaskRunArtifactsFinalizeUploadRequest { /** Array of uploaded artifacts to finalize */ artifacts: TaskRunArtifactFinalizeUpload[];