From 7557ab13abbf12f3d295fd8435b83fd8ee4b93f4 Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Mon, 25 May 2026 16:33:03 +0200 Subject: [PATCH] fix(api): delete agent threads with sequential cleanup Remove attachments and soft-delete the thread before storage cleanup so partial failures do not leave inconsistent DB state. Log storage errors without failing the request after the thread is marked deleted. Co-authored-by: Cursor --- .../api/src/features/agent/service.test.ts | 34 +++++++++++++++++++ packages/api/src/features/agent/service.ts | 23 ++++++++----- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/api/src/features/agent/service.test.ts b/packages/api/src/features/agent/service.test.ts index d8a7a0491..2c60738a5 100644 --- a/packages/api/src/features/agent/service.test.ts +++ b/packages/api/src/features/agent/service.test.ts @@ -1194,10 +1194,44 @@ describe("agentService.threads.delete", () => { await agentService.threads.delete({ id: "thread-own", userId: "user-own" }); + expect(dbMock.delete).toHaveBeenCalledBefore(storageServiceMock.delete as never); + expect(updateSet).toHaveBeenCalledBefore(storageServiceMock.delete as never); expect(storageServiceMock.delete).toHaveBeenCalledWith("uploads/user-own/agent/thread-own"); expect(dbMock.delete).toHaveBeenCalled(); expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ status: "deleted" })); }); + + it("still completes when storage cleanup fails after the thread is soft-deleted", async () => { + const ownedThread = buildArchivedThread({ + id: "thread-own", + userId: "user-own", + status: "active", + activeRunId: null, + activeStreamId: null, + archivedAt: null, + }); + + dbMock.select.mockImplementation(() => { + const limit = vi.fn(async () => [ownedThread]); + const where = vi.fn(() => ({ limit })); + const from = vi.fn(() => ({ where })); + return { from }; + }); + + const deleteWhere = vi.fn(async () => undefined); + dbMock.delete.mockReturnValue({ where: deleteWhere }); + + const updateWhere = vi.fn(async () => undefined); + const updateSet = vi.fn(() => ({ where: updateWhere })); + dbMock.update.mockReturnValue({ set: updateSet }); + + storageServiceMock.delete.mockRejectedValue(new Error("storage unavailable")); + + const { agentService } = await import("./service"); + + await expect(agentService.threads.delete({ id: "thread-own", userId: "user-own" })).resolves.toBeUndefined(); + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ status: "deleted" })); + }); }); describe("agentService.actions.revert", () => { diff --git a/packages/api/src/features/agent/service.ts b/packages/api/src/features/agent/service.ts index 5db86317d..e4f17a69b 100644 --- a/packages/api/src/features/agent/service.ts +++ b/packages/api/src/features/agent/service.ts @@ -946,14 +946,21 @@ export const agentService = { await getThread({ id: input.id, userId: input.userId }); - await Promise.all([ - getStorageService().delete(`uploads/${input.userId}/agent/${input.id}`), - db.delete(schema.agentAttachment).where(eq(schema.agentAttachment.threadId, input.id)), - db - .update(schema.agentThread) - .set({ status: "deleted", deletedAt: new Date() }) - .where(and(eq(schema.agentThread.id, input.id), eq(schema.agentThread.userId, input.userId))), - ]); + await db.delete(schema.agentAttachment).where(eq(schema.agentAttachment.threadId, input.id)); + await db + .update(schema.agentThread) + .set({ status: "deleted", deletedAt: new Date() }) + .where(and(eq(schema.agentThread.id, input.id), eq(schema.agentThread.userId, input.userId))); + + try { + await getStorageService().delete(`uploads/${input.userId}/agent/${input.id}`); + } catch (error) { + console.error("[agent] Failed to delete thread storage after soft-delete", { + threadId: input.id, + userId: input.userId, + error, + }); + } }, },