Skip to content

Commit 9e9fac3

Browse files
committed
fix(cleanup): purge chat inline images by prefix instead of by transcript
Inline chat images live at `chat-images/<chatId>/<requestId>/<sha>.webp` with no `workspace_files` row, and chat cleanup found them by re-deriving keys from the transcript it read before the delete. An image a turn published after that read, which the admin removal hook and the account deletion pre-read both leave a window for, was never collected and stayed in storage. The `copilot.chat.backend-cleanup` handler now deletes every object under `chat-images/<chatId>/` for each chat it confirms deleted, through the same paged listing and deadline-aware step the Project prefix cleanup uses (moved into `lib/uploads/core/sweep-prefix.ts` and shared), checkpointing how many chats it finished and continuing in a later run near its deadline. `collectChatFiles` no longer derives inline image keys and reads only each message's `fileAttachments`. So that every path still reaches the sweep, chat ids are queued even without `COPILOT_API_KEY`, with a `copilotBackend` flag saying whether they also go to the backend (absent on already-queued events, which keeps their meaning), and the retention jobs behind `prepareChatCleanup` queue an image-only event for the chats they confirm deleted. Events already queued with inline image file entries still delete those keys normally. Comments on the account deletion and admin removal paths now state the residual precisely: chats created after the read, and attachments added to an already-read chat. Autopsy: the tests seeded each chat's images together with the transcript that names them, so collecting keys from the transcript always matched what was stored; none wrote an object after the cleanup read, which is the window the two removal paths leave open. The new integration case writes images after the admin hook's read and fails on the old handler with the late images still on disk; a bystander chat's images stay untouched.
1 parent b53b02e commit 9e9fac3

11 files changed

Lines changed: 209 additions & 143 deletions

‎apps/sim/lib/auth/admin-removal-chat-cleanup.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ const ADMIN_REMOVAL_CLEANUP_DELAY_MS = 60 * 60_000
1919
* queued now, before the delete, and delayed past it. The handler purges only chats whose rows
2020
* are gone, so a delete that fails or is refused purges nothing. A failed read or enqueue throws,
2121
* which aborts the removal rather than erasing chats whose cleanup nothing recorded. Better Auth's
22-
* delete runs outside any transaction this hook can join, so a chat or attachment written between
23-
* this read and that delete is not covered; account deletion, which owns its transaction, re-reads
24-
* under the account lock instead.
22+
* delete runs outside any transaction this hook can join, so what is written between this read
23+
* and that delete is covered only in part: a chat created in that window is not purged at all, and
24+
* an attachment (a `workspace_files` row or message key) added to a chat already read stays in
25+
* storage, while that chat's inline images still go, because the handler sweeps each read chat's
26+
* whole image prefix. Account deletion, which owns its transaction, re-reads under the account
27+
* lock instead.
2528
*/
2629
export async function enqueueAdminRemovedChatCleanup(user: { id: string }): Promise<void> {
2730
const targets = await readAccountRemovalChats({

‎apps/sim/lib/cleanup/chat-backend-cleanup-outbox.ts‎

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,67 @@ import {
99
organizationAttachmentsStillReferenced,
1010
} from '@/lib/cleanup/chat-cleanup'
1111
import { env } from '@/lib/core/config/env'
12-
import type { OutboxHandlerRegistry } from '@/lib/core/outbox/service'
12+
import {
13+
continueOutboxHandler,
14+
deferOutboxHandler,
15+
type OutboxEventContext,
16+
type OutboxHandlerRegistry,
17+
} from '@/lib/core/outbox/service'
1318
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
19+
import {
20+
INLINE_CHAT_IMAGE_PREFIX,
21+
inlineImageRequestIdSchema,
22+
} from '@/lib/mothership/chat/inline-image-reference'
23+
import { outboxDeadlineNear } from '@/lib/uploads/core/prefix-sweep'
1424
import { deleteFile } from '@/lib/uploads/core/storage-service'
25+
import { sweepStoragePrefix } from '@/lib/uploads/core/sweep-prefix'
1526

1627
const FILE_DELETE_CONCURRENCY = 10
1728

29+
/**
30+
* Deletes every inline image under each deleted chat's `chat-images/<chatId>/` prefix, from the
31+
* `from`th chat on. `swept` is how many leading chats are done: all of them unless the deadline
32+
* neared (`continue`) or a listing stopped advancing (`stalled`).
33+
*/
34+
async function sweepInlineImages(
35+
chatIds: readonly string[],
36+
deleted: ReadonlySet<string>,
37+
from: number,
38+
context: OutboxEventContext
39+
): Promise<{ swept: number; outcome: 'done' | 'continue' | 'stalled' }> {
40+
for (let index = from; index < chatIds.length; index++) {
41+
const chatId = chatIds[index]
42+
// A chat id outside the image key grammar never had an image stored under it.
43+
if (!deleted.has(chatId) || !inlineImageRequestIdSchema.safeParse(chatId).success) continue
44+
for (;;) {
45+
context.signal.throwIfAborted()
46+
const outcome = await sweepStoragePrefix(
47+
`${INLINE_CHAT_IMAGE_PREFIX}${chatId}/`,
48+
'mothership',
49+
context
50+
)
51+
if (outcome === 'done') break
52+
if (outcome === 'stalled' || outboxDeadlineNear(context)) return { swept: index, outcome }
53+
}
54+
if (outboxDeadlineNear(context)) return { swept: index + 1, outcome: 'continue' }
55+
}
56+
return { swept: chatIds.length, outcome: 'done' }
57+
}
58+
1859
export const chatBackendCleanupOutboxHandlers = {
1960
/**
20-
* Purges the copilot backend data and stored files of chats whose rows are gone. A chat whose
21-
* row survived (its delete was spared, rolled back, or never happened) keeps both. Any failure
22-
* throws so the outbox retries it: a failed backend call, a failed object delete, or chat ids
23-
* queued while `COPILOT_API_KEY` was set reaching a processor without it. An object already
24-
* gone counts as deleted, so a retry repeats only what is left.
61+
* Purges the inline images, copilot backend data and stored files of chats whose rows are gone.
62+
* A chat whose row survived (its delete was spared, rolled back, or never happened) keeps all
63+
* three. Inline images go by the chat's prefix rather than by key, so one a turn published after
64+
* the delete read the transcript goes too; the sweep checkpoints how many chats it finished and
65+
* continues in a later run once its deadline nears. Any failure throws so the outbox retries it:
66+
* a failed backend call, a failed object delete, or chat ids queued for the backend reaching a
67+
* processor without `COPILOT_API_KEY`. An object already gone counts as deleted, so a retry
68+
* repeats only what is left.
2569
*/
2670
[CHAT_BACKEND_CLEANUP_EVENT]: async (rawPayload, context) => {
27-
const { chatIds, files } = chatBackendCleanupPayloadSchema.parse(rawPayload)
71+
const { chatIds, files, copilotBackend, imagesSwept } =
72+
chatBackendCleanupPayloadSchema.parse(rawPayload)
2873
context.signal.throwIfAborted()
2974
const named = [...new Set([...chatIds, ...files.map((file) => file.chatId)])]
3075
const surviving = await db
@@ -35,7 +80,16 @@ export const chatBackendCleanupOutboxHandlers = {
3580
const deletedChatIds = chatIds.filter((id) => !survivingIds.has(id))
3681
const deletedFiles = files.filter((file) => !survivingIds.has(file.chatId))
3782

38-
if (deletedChatIds.length > 0) {
83+
if (imagesSwept < chatIds.length) {
84+
const sweep = await sweepInlineImages(chatIds, new Set(deletedChatIds), imagesSwept, context)
85+
if (sweep.swept > imagesSwept) await context.checkpointPayload({ imagesSwept: sweep.swept })
86+
if (sweep.outcome === 'continue')
87+
return continueOutboxHandler('Continuing chat inline image cleanup')
88+
if (sweep.outcome === 'stalled')
89+
return deferOutboxHandler('Chat inline image cleanup removed nothing from its listing')
90+
}
91+
92+
if (copilotBackend && deletedChatIds.length > 0) {
3993
context.signal.throwIfAborted()
4094
if (!env.COPILOT_API_KEY) throw new Error('Copilot cleanup is not configured')
4195
const result = await cleanupCopilotBackend(

‎apps/sim/lib/cleanup/chat-backend-cleanup.integration.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ import { chatBackendCleanupOutboxHandlers } from '@/lib/cleanup/chat-backend-cle
8181
import { env } from '@/lib/core/config/env'
8282
import { processOutboxEventById } from '@/lib/core/outbox/service'
8383
import { deleteCopilotChat } from '@/lib/mothership/chat/deletion'
84+
import { inlineChatImageKey } from '@/lib/mothership/chat/inline-image-key'
8485
import { deleteUserAccount } from '@/lib/users/account-deletion'
8586

8687
setUploadDirServer(storageRoot)
@@ -434,6 +435,58 @@ describe('cleanup of the chats a removal cascades away', () => {
434435
expect(await objectExists(ownChat.keys.attachment)).toBe(false)
435436
})
436437

438+
/**
439+
* Inline images have no row, and the hook reads the transcript before Better Auth deletes the
440+
* user, so an image a turn publishes in between was never collected by key.
441+
*/
442+
it('purges inline images a removed user’s chat published after the cleanup read it', async () => {
443+
const removedId = await seedUser()
444+
const bystanderId = await seedUser()
445+
const workspaceId = await seedWorkspace(bystanderId, [removedId])
446+
const chatId = await seedChat(removedId, workspaceId)
447+
const bystandersChat = await seedChat(bystanderId, workspaceId)
448+
const read = inlineChatImageKey(chatId, 'req-1', 'files/chart.png')
449+
await db.insert(copilotMessages).values({
450+
chatId,
451+
messageId: generateId(),
452+
role: 'assistant',
453+
content: { role: 'assistant', requestId: 'req-1', content: '![chart](files/chart.png)' },
454+
})
455+
await putObject(read)
456+
const bystanders = inlineChatImageKey(bystandersChat, 'req-1', 'files/chart.png')
457+
await putObject(bystanders)
458+
459+
await enqueueAdminRemovedChatCleanup({ id: removedId })
460+
const late = [
461+
inlineChatImageKey(chatId, 'req-2', 'files/later.png'),
462+
inlineChatImageKey(chatId, 'req-2', 'files/another.png'),
463+
]
464+
for (const key of late) await putObject(key)
465+
await db.delete(user).where(eq(user.id, removedId))
466+
467+
const delivered = await deliverCleanups()
468+
expect(new Set(delivered.outcomes)).toEqual(new Set(['completed']))
469+
for (const key of [read, ...late]) expect(await objectExists(key)).toBe(false)
470+
expect(await objectExists(bystanders)).toBe(true)
471+
})
472+
473+
it('still deletes the inline image keys an event queued before prefix sweeps names', async () => {
474+
const ownerId = await seedUser()
475+
const workspaceId = await seedWorkspace(ownerId)
476+
const chatId = await seedChat(ownerId, workspaceId)
477+
const key = inlineChatImageKey(chatId, 'req-1', 'files/chart.png')
478+
await putObject(key)
479+
await db.delete(copilotChats).where(eq(copilotChats.id, chatId))
480+
await db.insert(outboxEvent).values({
481+
id: generateId(),
482+
eventType: CHAT_BACKEND_CLEANUP_EVENT,
483+
payload: { chatIds: [], files: [{ key, context: 'mothership', chatId }] },
484+
})
485+
486+
expect((await deliverCleanups()).outcomes).toEqual(['completed'])
487+
expect(await objectExists(key)).toBe(false)
488+
})
489+
437490
it('purges nothing for a direct user delete that never happened', async () => {
438491
const keptId = await seedUser()
439492
const workspaceId = await seedWorkspace(keptId)

‎apps/sim/lib/cleanup/chat-backend-cleanup.ts‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,17 @@ export const chatBackendCleanupPayloadSchema = z
3434
.object({
3535
chatIds: z.array(chatIdSchema).max(CHAT_BACKEND_CLEANUP_BATCH_SIZE).default([]),
3636
files: z.array(chatFileSchema).max(CHAT_BACKEND_CLEANUP_BATCH_SIZE).default([]),
37+
/** Whether `chatIds` also reach the copilot backend; an event without it means they do. */
38+
copilotBackend: z.boolean().default(true),
39+
/** How many leading `chatIds` already had their inline image prefix swept; a checkpoint. */
40+
imagesSwept: z.number().int().min(0).max(CHAT_BACKEND_CLEANUP_BATCH_SIZE).default(0),
3741
})
3842
.strict()
3943
.refine((payload) => payload.chatIds.length > 0 || payload.files.length > 0, {
4044
message: 'A chat cleanup names at least one chat or stored file',
4145
})
4246

43-
type ChatBackendCleanupPayload = z.infer<typeof chatBackendCleanupPayloadSchema>
47+
type ChatBackendCleanupPayload = z.input<typeof chatBackendCleanupPayloadSchema>
4448

4549
/** What a chat deletion leaves outside Sim's rows: the chats' copilot backend data and stored files. */
4650
export interface ChatCleanupTargets {
@@ -51,27 +55,31 @@ export interface ChatCleanupTargets {
5155

5256
/**
5357
* Queues the purge of what the given chats leave outside Sim's rows once a delete cascades them
54-
* away: one bounded event per {@link CHAT_BACKEND_CLEANUP_BATCH_SIZE} chats for the copilot backend,
55-
* and one per as many stored files. Call it in the deleting transaction so the cleanup commits or
56-
* rolls back with the delete; the handler purges only chats whose rows are gone, so a chat the
57-
* delete ends up sparing keeps its data. A delete with no transaction to join queues it first,
58-
* with an `availableAt` past the moment the delete commits.
58+
* away: one bounded event per {@link CHAT_BACKEND_CLEANUP_BATCH_SIZE} chats, which sweeps each
59+
* chat's inline image prefix and purges its copilot backend data, and one per as many stored
60+
* files. Call it in the deleting transaction so the cleanup commits or rolls back with the delete;
61+
* the handler purges only chats whose rows are gone, so a chat the delete ends up sparing keeps its
62+
* data. A delete with no transaction to join queues it first, with an `availableAt` past the
63+
* moment the delete commits.
5964
*
60-
* Without `COPILOT_API_KEY` this Sim never reached a copilot backend, so no chat ids are queued for
61-
* it; the stored files are queued either way.
65+
* Without `COPILOT_API_KEY` this Sim never reached a copilot backend, so the chats are queued for
66+
* their image sweep only; `copilotBackend: false` does the same for a caller that already purged
67+
* the backend itself.
6268
*/
6369
export async function enqueueChatBackendCleanup(
6470
executor: Pick<typeof db, 'insert'>,
6571
targets: ChatCleanupTargets,
66-
options: { availableAt?: Date } = {}
72+
options: { availableAt?: Date; copilotBackend?: boolean } = {}
6773
): Promise<void> {
68-
const chatIds = env.COPILOT_API_KEY ? [...new Set(targets.chatIds)] : []
74+
const chatIds = [...new Set(targets.chatIds)]
75+
const copilotBackend = (options.copilotBackend ?? true) && Boolean(env.COPILOT_API_KEY)
6976
const filesByKey = new Map<string, ChatFileRef>()
7077
for (const file of targets.files) if (!filesByKey.has(file.key)) filesByKey.set(file.key, file)
7178
const payloads: ChatBackendCleanupPayload[] = [
7279
...chunkArray(chatIds, CHAT_BACKEND_CLEANUP_BATCH_SIZE).map((batch) => ({
7380
chatIds: batch,
7481
files: [],
82+
copilotBackend,
7583
})),
7684
...chunkArray([...filesByKey.values()], CHAT_BACKEND_CLEANUP_BATCH_SIZE).map((batch) => ({
7785
chatIds: [],
@@ -84,7 +92,9 @@ export async function enqueueChatBackendCleanup(
8492
})),
8593
]
8694
for (const events of chunkArray(payloads, MAX_BULK_ENQUEUE_EVENTS)) {
87-
await enqueueOutboxEvents(executor, CHAT_BACKEND_CLEANUP_EVENT, events, options)
95+
await enqueueOutboxEvents(executor, CHAT_BACKEND_CLEANUP_EVENT, events, {
96+
availableAt: options.availableAt,
97+
})
8898
}
8999
}
90100

@@ -104,9 +114,11 @@ export async function readAccountRemovalChats(params: {
104114
/**
105115
* Queues the cleanup of every chat an account removal cascades away, inside its transaction:
106116
* the chat ids are re-read there, after the account row is locked, and the files of any chat
107-
* created since {@link readAccountRemovalChats} ran are read in the transaction. A key a turn adds
108-
* to an already-read chat between that read and the removal's run stops is not collected here;
109-
* when it names one of the account's uploads, the account storage sweep still reclaims it.
117+
* created since {@link readAccountRemovalChats} ran are read in the transaction. Every chat's inline
118+
* images go with its prefix sweep, however late they were written. An attachment a turn adds to
119+
* an already-read chat between that read and the removal's run stops is not collected here; when
120+
* it names one of the account's uploads, the account storage sweep still reclaims it, and
121+
* otherwise it stays behind.
110122
*/
111123
export async function enqueueAccountRemovalChatCleanupInTx(
112124
tx: DbTransaction,

‎apps/sim/lib/cleanup/chat-cleanup.integration.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { compareStrings } from '@sim/utils/string'
66
import { eq } from 'drizzle-orm'
77
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
88
import { collectChatFiles } from '@/lib/cleanup/chat-files'
9-
import { inlineChatImageKey } from '@/lib/mothership/chat/inline-image-key'
109

1110
function attachment(key: string) {
1211
return { id: generateId(), key, filename: 'x.png', media_type: 'image/png', size: 1 }
@@ -51,7 +50,7 @@ describe('chat cleanup file collection in PostgreSQL', () => {
5150
return chat.id
5251
}
5352

54-
it('collects attachment and inline image keys from every row, never from tool payloads', async () => {
53+
it('collects attachment keys from every row, never from tool payloads or inline images', async () => {
5554
const chatId = await seedChat(
5655
[
5756
{
@@ -110,12 +109,6 @@ describe('chat cleanup file collection in PostgreSQL', () => {
110109
chatId,
111110
organizationId: 'org-1',
112111
},
113-
{
114-
key: inlineChatImageKey(chatId, 'req-1', 'files/chart.png'),
115-
context: 'mothership',
116-
chatId,
117-
},
118-
{ key: inlineChatImageKey(chatId, 'req-1', '/tmp/out.png'), context: 'mothership', chatId },
119112
{ key: 'copilot/1234/legacy.png', context: 'copilot', chatId },
120113
{ key: 'copilot/5/other.png', context: 'copilot', chatId: otherChatId },
121114
{ key: 'copilot/9/removed.png', context: 'copilot', chatId },

‎apps/sim/lib/cleanup/chat-cleanup.test.ts‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
77
vi.mock('@/lib/uploads', () => uploadsMock)
88

99
import { prepareChatCleanup } from '@/lib/cleanup/chat-cleanup'
10-
import { inlineChatImageKey } from '@/lib/mothership/chat/inline-image-key'
1110

1211
const chatId = '3f0c2a52-8a43-4d4b-9b5f-0b3c7a1e2d10'
1312

@@ -26,7 +25,7 @@ async function purge(
2625
queueTableRows(workspaceFiles, [])
2726
queueTableRows(
2827
copilotMessages,
29-
messages.map((content) => ({ chatId, content }))
28+
messages.map((content) => ({ chatId, fileAttachments: content.fileAttachments ?? null }))
3029
)
3130
const cleanup = await prepareChatCleanup([chatId], 'test')
3231
queueTableRows(copilotChats, [])
@@ -90,23 +89,4 @@ describe('chat purge storage', () => {
9089
])
9190
expect(deleted).toEqual({ copilot: ['copilot/1234/kept.png'] })
9291
})
93-
94-
it('deletes the inline images an assistant message published', async () => {
95-
const deleted = await purge([
96-
{
97-
role: 'assistant',
98-
requestId: 'req-1',
99-
content: 'Here: ![chart](files/chart.png) and `![code](files/no.png)`',
100-
contentBlocks: [{ type: 'text', content: '![other](/tmp/out.png)' }],
101-
},
102-
{ role: 'user', requestId: 'req-2', content: '![u](files/u.png)' },
103-
{ role: 'assistant', content: '![x](files/x.png)' },
104-
])
105-
expect(deleted).toEqual({
106-
mothership: [
107-
inlineChatImageKey(chatId, 'req-1', 'files/chart.png'),
108-
inlineChatImageKey(chatId, 'req-1', '/tmp/out.png'),
109-
],
110-
})
111-
})
11292
})

‎apps/sim/lib/cleanup/chat-cleanup.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger'
44
import { chunkArray } from '@sim/utils/helpers'
55
import { isRecordLike } from '@sim/utils/object'
66
import { and, eq, inArray, or, sql } from 'drizzle-orm'
7+
import { enqueueChatBackendCleanup } from '@/lib/cleanup/chat-backend-cleanup'
78
import {
89
type ChatFileRef,
910
type ChatScopedContext,
@@ -156,7 +157,7 @@ export async function cleanupCopilotBackend(
156157

157158
/**
158159
* Full chat cleanup: collect file refs, then (after DB deletion by caller)
159-
* call copilot backend and delete storage files.
160+
* call copilot backend, delete storage files, and queue the chats' inline image sweep.
160161
*
161162
* Usage:
162163
* const cleanup = await prepareChatCleanup(chatIds, label)
@@ -207,6 +208,12 @@ export async function prepareChatCleanup(
207208
logger.info(
208209
`[${label}] Copilot backend: ${copilotResult.deleted} deleted, ${copilotResult.failed} failed`
209210
)
211+
// Inline images have no row or key list to delete by; the outbox sweeps each chat's prefix.
212+
await enqueueChatBackendCleanup(
213+
cleanupDb,
214+
{ chatIds: confirmedChatIds, files: [] },
215+
{ copilotBackend: false }
216+
)
210217
}
211218

212219
// Delete storage files with correct context per file

0 commit comments

Comments
 (0)