From 317c082ba159582b9423873195d53d9adc720293 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Fri, 4 Sep 2026 05:20:26 +0000 Subject: [PATCH] fix: preserve artifacts when task deletion fails --- .../commands/tasks/__tests__/delete.test.ts | 24 +++++++++++++++ apps/web/src/trpc/commands/tasks/delete.ts | 30 ++++++++----------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/apps/web/src/trpc/commands/tasks/__tests__/delete.test.ts b/apps/web/src/trpc/commands/tasks/__tests__/delete.test.ts index a60d259ca..8edad4b0f 100644 --- a/apps/web/src/trpc/commands/tasks/__tests__/delete.test.ts +++ b/apps/web/src/trpc/commands/tasks/__tests__/delete.test.ts @@ -168,6 +168,30 @@ describe('deleteTasksCommand', () => { ); }); + it('preserves task metadata when artifact deletion has partial errors', async () => { + mockDeleteArtifactsBatch.mockResolvedValue({ deleted: 0, errors: 1 }); + + await expect( + deleteTasksCommand(auth, { taskIds: ['task-1'] }), + ).rejects.toThrow('Failed to delete 1 artifact objects for tasks: task-1'); + + expect(deleteCalls).toHaveLength(0); + expect(updateCalls).toHaveLength(0); + expect(mockMarkParallelCounts).not.toHaveBeenCalled(); + }); + + it('preserves task metadata when artifact deletion throws', async () => { + mockDeleteArtifactsBatch.mockRejectedValue(new Error('S3 unavailable')); + + await expect( + deleteTasksCommand(auth, { taskIds: ['task-1'] }), + ).rejects.toThrow('S3 unavailable'); + + expect(deleteCalls).toHaveLength(0); + expect(updateCalls).toHaveLength(0); + expect(mockMarkParallelCounts).not.toHaveBeenCalled(); + }); + it('archives a session left with no live tasks', async () => { await deleteTasksCommand(auth, { taskIds: ['task-1'] }); diff --git a/apps/web/src/trpc/commands/tasks/delete.ts b/apps/web/src/trpc/commands/tasks/delete.ts index 1883e684d..ad0e5c66f 100644 --- a/apps/web/src/trpc/commands/tasks/delete.ts +++ b/apps/web/src/trpc/commands/tasks/delete.ts @@ -55,27 +55,23 @@ export async function deleteTasksCommand( .from(taskArtifacts) .where(inArray(taskArtifacts.taskId, taskIdsToDelete)); - // Delete S3 objects for these artifacts (best-effort). + // Delete S3 objects before removing their retry metadata. let s3Result = { deleted: 0, errors: 0 }; if (artifactsToDelete.length > 0) { - try { - s3Result = await deleteArtifactsBatch( - artifactsToDelete.map((artifact) => ({ - taskId: artifact.taskId!, - artifactId: artifact.id, - path: artifact.path, - version: artifact.version, - })), + s3Result = await deleteArtifactsBatch( + artifactsToDelete.map((artifact) => ({ + taskId: artifact.taskId!, + artifactId: artifact.id, + path: artifact.path, + version: artifact.version, + })), + ); + + if (s3Result.errors > 0) { + throw new Error( + `Failed to delete ${s3Result.errors} artifact objects for tasks: ${taskIdsToDelete.join(', ')}`, ); - - if (s3Result.errors > 0) { - console.warn( - `[deleteTasksCommand] S3 deletion had ${s3Result.errors} errors for tasks: ${taskIdsToDelete.join(', ')}`, - ); - } - } catch (s3Error) { - console.error('[deleteTasksCommand] S3 deletion error:', s3Error); } }