Skip to content

Commit 5af9040

Browse files
committed
improvement(cleanup): sweep chat inline images through their own outbox event
Inline image cleanup gets CHAT_INLINE_IMAGE_CLEANUP_EVENT ({ chatIds }) and its own handler: it sweeps the chat-images/<chatId>/ prefix of each chat whose row is gone, ten chats at a time, checkpoints how many leading chats are finished (the lowest unfinished one), and continues or defers past its deadline. enqueueChatBackendCleanup and prepareChatCleanup each queue it for the chats they cover; the backend event drops its copilotBackend flag and imagesSwept checkpoint and keeps accepting the shipped { chatIds, files } payload. Completed image events are pruned with the backend ones. The reference check behind deletableChatFiles now runs one query per scope that returns only the matched keys (passed as one JSON parameter), after an EXISTS that skips a scope with no chat left. ChatFileRef carries one optional scope (absent on legacy events, null when no other chat can reference the key); the event schema still maps a legacy organizationId to it. Legacy key-scope recovery reuses extractWorkspaceIdFromStorageKey, and inlineChatImagePrefix validates the chat id for both the key and the sweep.
1 parent 131f0d3 commit 5af9040

14 files changed

Lines changed: 574 additions & 301 deletions

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,9 @@ import {
1414
const ADMIN_REMOVAL_CLEANUP_DELAY_MS = 60 * 60_000
1515

1616
/**
17-
* The user delete's `before` hook: `/admin/remove-user` cascades the account's chats, and those in
18-
* the workspaces it owns, away with no transaction to queue their cleanup in, so the cleanup is
19-
* queued now, before the delete, and delayed past it. The handler purges only chats whose rows
20-
* are gone, so a delete that fails or is refused purges nothing. A failed read or enqueue throws,
21-
* 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 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.
17+
* The user delete's `before` hook: queues, delayed past the delete, the cleanup of the chats
18+
* `/admin/remove-user` cascades away; a failed read or enqueue throws and aborts the removal. A
19+
* chat or attachment written between this read and Better Auth's delete is not covered.
2820
*/
2921
export async function enqueueAdminRemovedChatCleanup(user: { id: string }): Promise<void> {
3022
const targets = await readAccountRemovalChats({
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
export const CHAT_BACKEND_CLEANUP_EVENT = 'copilot.chat.backend-cleanup'
2+
export const CHAT_INLINE_IMAGE_CLEANUP_EVENT = 'copilot.chat.inline-image-cleanup'
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { copilotChats } from '@sim/db/schema'
2+
import { queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const { mockSweepStoragePrefix, deadline } = vi.hoisted(() => ({
6+
mockSweepStoragePrefix: vi.fn(),
7+
deadline: { near: false },
8+
}))
9+
10+
vi.mock('@/lib/uploads/core/prefix-sweep', () => ({
11+
sweepStoragePrefix: mockSweepStoragePrefix,
12+
outboxDeadlineNear: () => deadline.near,
13+
}))
14+
15+
import { CHAT_INLINE_IMAGE_CLEANUP_EVENT } from '@/lib/cleanup/chat-backend-cleanup-event'
16+
import { chatBackendCleanupOutboxHandlers } from '@/lib/cleanup/chat-backend-cleanup-outbox'
17+
import type { OutboxEventContext } from '@/lib/core/outbox/service'
18+
19+
const sweepImages = chatBackendCleanupOutboxHandlers[CHAT_INLINE_IMAGE_CLEANUP_EVENT]
20+
const chatIds = Array.from({ length: 25 }, (_, index) => `chat-${String(index).padStart(2, '0')}`)
21+
const prefixOf = (chatId: string) => `chat-images/${chatId}/`
22+
23+
function outboxContext() {
24+
return {
25+
eventId: 'event-1',
26+
eventType: CHAT_INLINE_IMAGE_CLEANUP_EVENT,
27+
attempts: 0,
28+
maxAttempts: 10,
29+
signal: new AbortController().signal,
30+
checkpointPayload: vi.fn(async (_patch: Record<string, unknown>) => {}),
31+
} satisfies OutboxEventContext
32+
}
33+
34+
const sweptPrefixes = () => mockSweepStoragePrefix.mock.calls.map(([prefix]) => prefix)
35+
36+
describe('chat inline image cleanup', () => {
37+
beforeEach(() => {
38+
resetDbChainMock()
39+
deadline.near = false
40+
mockSweepStoragePrefix.mockReset().mockResolvedValue('done')
41+
})
42+
43+
it('sweeps only the chats whose rows are gone', async () => {
44+
queueTableRows(copilotChats, [{ id: chatIds[1] }])
45+
46+
await expect(sweepImages({ chatIds: chatIds.slice(0, 3) }, outboxContext())).resolves.toBe(
47+
undefined
48+
)
49+
expect(sweptPrefixes().sort()).toEqual([prefixOf(chatIds[0]), prefixOf(chatIds[2])])
50+
})
51+
52+
it('sweeps several chats at once, never more than ten', async () => {
53+
queueTableRows(copilotChats, [])
54+
let inFlight = 0
55+
let peak = 0
56+
mockSweepStoragePrefix.mockImplementation(async () => {
57+
peak = Math.max(peak, ++inFlight)
58+
await new Promise((resolve) => setImmediate(resolve))
59+
inFlight--
60+
return 'done'
61+
})
62+
63+
await sweepImages({ chatIds }, outboxContext())
64+
65+
expect(peak).toBe(10)
66+
expect(new Set(sweptPrefixes())).toEqual(new Set(chatIds.map(prefixOf)))
67+
})
68+
69+
it('checkpoints the chats finished before the lowest unfinished one and resumes after them', async () => {
70+
queueTableRows(copilotChats, [])
71+
const slow = chatIds[3]
72+
mockSweepStoragePrefix.mockImplementation(async (prefix: string) => {
73+
if (prefix !== prefixOf(slow)) return 'done'
74+
deadline.near = true
75+
return 'continue'
76+
})
77+
const context = outboxContext()
78+
79+
const result = await sweepImages({ chatIds: chatIds.slice(0, 6) }, context)
80+
81+
expect(result).toMatchObject({ outcome: 'deferred', consumeAttempt: false })
82+
expect(context.checkpointPayload).toHaveBeenCalledWith({ swept: 3 })
83+
84+
queueTableRows(copilotChats, [])
85+
deadline.near = false
86+
mockSweepStoragePrefix.mockReset().mockResolvedValue('done')
87+
await expect(
88+
sweepImages({ chatIds: chatIds.slice(0, 6), swept: 3 }, outboxContext())
89+
).resolves.toBe(undefined)
90+
expect(sweptPrefixes().sort()).toEqual(chatIds.slice(3, 6).map(prefixOf))
91+
})
92+
93+
it('spends an attempt when a chat’s listing stops advancing', async () => {
94+
queueTableRows(copilotChats, [])
95+
mockSweepStoragePrefix.mockImplementation(async (prefix: string) =>
96+
prefix === prefixOf(chatIds[0]) ? 'stalled' : 'done'
97+
)
98+
const context = outboxContext()
99+
100+
const result = await sweepImages({ chatIds: chatIds.slice(0, 2) }, context)
101+
102+
expect(result).toMatchObject({ outcome: 'deferred' })
103+
expect(result).not.toMatchObject({ consumeAttempt: false })
104+
expect(context.checkpointPayload).not.toHaveBeenCalled()
105+
})
106+
})

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

Lines changed: 64 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@ import { db } from '@sim/db'
22
import { copilotChats } from '@sim/db/schema'
33
import { describeError, toError } from '@sim/utils/errors'
44
import { inArray } from 'drizzle-orm'
5-
import { chatBackendCleanupPayloadSchema } from '@/lib/cleanup/chat-backend-cleanup'
6-
import { CHAT_BACKEND_CLEANUP_EVENT } from '@/lib/cleanup/chat-backend-cleanup-event'
5+
import {
6+
chatBackendCleanupPayloadSchema,
7+
chatInlineImageCleanupPayloadSchema,
8+
} from '@/lib/cleanup/chat-backend-cleanup'
9+
import {
10+
CHAT_BACKEND_CLEANUP_EVENT,
11+
CHAT_INLINE_IMAGE_CLEANUP_EVENT,
12+
} from '@/lib/cleanup/chat-backend-cleanup-event'
713
import { cleanupCopilotBackend, deletableChatFiles } from '@/lib/cleanup/chat-cleanup'
814
import { env } from '@/lib/core/config/env'
915
import {
@@ -13,80 +19,57 @@ import {
1319
type OutboxHandlerRegistry,
1420
} from '@/lib/core/outbox/service'
1521
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
16-
import {
17-
INLINE_CHAT_IMAGE_PREFIX,
18-
inlineImageRequestIdSchema,
19-
} from '@/lib/mothership/chat/inline-image-reference'
22+
import { inlineChatImagePrefix } from '@/lib/mothership/chat/inline-image-key'
2023
import { outboxDeadlineNear, sweepStoragePrefix } from '@/lib/uploads/core/prefix-sweep'
2124
import { deleteFile } from '@/lib/uploads/core/storage-service'
2225

23-
const FILE_DELETE_CONCURRENCY = 10
26+
/** Most object deletes, or chat prefix sweeps, one handler run has in flight at once. */
27+
const STORAGE_CONCURRENCY = 10
28+
29+
/** The ids among `chatIds` that still have a `copilot_chats` row. */
30+
async function survivingChatIds(chatIds: readonly string[]): Promise<Set<string>> {
31+
if (chatIds.length === 0) return new Set()
32+
const rows = await db
33+
.select({ id: copilotChats.id })
34+
.from(copilotChats)
35+
.where(inArray(copilotChats.id, [...chatIds]))
36+
return new Set(rows.map(({ id }) => id))
37+
}
2438

2539
/**
26-
* Deletes every inline image under each deleted chat's `chat-images/<chatId>/` prefix, from the
27-
* `from`th chat on. `swept` is how many leading chats are done: all of them unless the deadline
28-
* neared (`continue`) or a listing stopped advancing (`stalled`).
40+
* Sweeps `chatId`'s inline image prefix until it is empty, the deadline nears (`continue`), or a
41+
* listing stops advancing (`stalled`).
2942
*/
30-
async function sweepInlineImages(
31-
chatIds: readonly string[],
32-
deleted: ReadonlySet<string>,
33-
from: number,
43+
async function sweepChatImages(
44+
chatId: string,
3445
context: OutboxEventContext
35-
): Promise<{ swept: number; outcome: 'done' | 'continue' | 'stalled' }> {
36-
for (let index = from; index < chatIds.length; index++) {
37-
const chatId = chatIds[index]
38-
// A chat id outside the image key grammar never had an image stored under it.
39-
if (!deleted.has(chatId) || !inlineImageRequestIdSchema.safeParse(chatId).success) continue
40-
for (;;) {
41-
context.signal.throwIfAborted()
42-
const outcome = await sweepStoragePrefix(
43-
`${INLINE_CHAT_IMAGE_PREFIX}${chatId}/`,
44-
'mothership',
45-
context
46-
)
47-
if (outcome === 'done') break
48-
if (outcome === 'stalled' || outboxDeadlineNear(context)) return { swept: index, outcome }
49-
}
50-
if (outboxDeadlineNear(context)) return { swept: index + 1, outcome: 'continue' }
46+
): Promise<'done' | 'continue' | 'stalled'> {
47+
const prefix = inlineChatImagePrefix(chatId)
48+
for (;;) {
49+
context.signal.throwIfAborted()
50+
if (outboxDeadlineNear(context)) return 'continue'
51+
const outcome = await sweepStoragePrefix(prefix, 'mothership', context)
52+
if (outcome !== 'continue') return outcome
5153
}
52-
return { swept: chatIds.length, outcome: 'done' }
5354
}
5455

5556
export const chatBackendCleanupOutboxHandlers = {
5657
/**
57-
* Purges the inline images, copilot backend data and stored files of chats whose rows are gone.
58-
* A chat whose row survived (its delete was spared, rolled back, or never happened) keeps all
59-
* three. Inline images go by the chat's prefix rather than by key, so one a turn published after
60-
* the delete read the transcript goes too; the sweep checkpoints how many chats it finished and
61-
* continues in a later run once its deadline nears. Any failure throws so the outbox retries it:
62-
* a failed backend call, a failed object delete, or chat ids queued for the backend reaching a
63-
* processor without `COPILOT_API_KEY`. An object already gone counts as deleted, so a retry
64-
* repeats only what is left. A stored file another chat of its scope still references, as a
65-
* fork's uncopied attachment does, is kept ({@link deletableChatFiles}).
58+
* Purges the copilot backend data and stored files of chats whose rows are gone; a surviving
59+
* chat keeps both, and a file another chat of its scope still references is kept
60+
* ({@link deletableChatFiles}). Any failure throws so the outbox retries; an object already gone
61+
* counts as deleted.
6662
*/
6763
[CHAT_BACKEND_CLEANUP_EVENT]: async (rawPayload, context) => {
68-
const { chatIds, files, copilotBackend, imagesSwept } =
69-
chatBackendCleanupPayloadSchema.parse(rawPayload)
64+
const { chatIds, files } = chatBackendCleanupPayloadSchema.parse(rawPayload)
7065
context.signal.throwIfAborted()
71-
const named = [...new Set([...chatIds, ...files.map((file) => file.chatId)])]
72-
const surviving = await db
73-
.select({ id: copilotChats.id })
74-
.from(copilotChats)
75-
.where(inArray(copilotChats.id, named))
76-
const survivingIds = new Set(surviving.map(({ id }) => id))
66+
const survivingIds = await survivingChatIds([
67+
...new Set([...chatIds, ...files.map((file) => file.chatId)]),
68+
])
7769
const deletedChatIds = chatIds.filter((id) => !survivingIds.has(id))
7870
const deletedFiles = files.filter((file) => !survivingIds.has(file.chatId))
7971

80-
if (imagesSwept < chatIds.length) {
81-
const sweep = await sweepInlineImages(chatIds, new Set(deletedChatIds), imagesSwept, context)
82-
if (sweep.swept > imagesSwept) await context.checkpointPayload({ imagesSwept: sweep.swept })
83-
if (sweep.outcome === 'continue')
84-
return continueOutboxHandler('Continuing chat inline image cleanup')
85-
if (sweep.outcome === 'stalled')
86-
return deferOutboxHandler('Chat inline image cleanup removed nothing from its listing')
87-
}
88-
89-
if (copilotBackend && deletedChatIds.length > 0) {
72+
if (deletedChatIds.length > 0) {
9073
context.signal.throwIfAborted()
9174
if (!env.COPILOT_API_KEY) throw new Error('Copilot cleanup is not configured')
9275
const result = await cleanupCopilotBackend(
@@ -99,7 +82,7 @@ export const chatBackendCleanupOutboxHandlers = {
9982
if (deletedFiles.length === 0) return
10083
const failures = await mapWithConcurrency(
10184
await deletableChatFiles(deletedFiles),
102-
FILE_DELETE_CONCURRENCY,
85+
STORAGE_CONCURRENCY,
10386
async ({ key, context: storageContext }) => {
10487
context.signal.throwIfAborted()
10588
try {
@@ -112,4 +95,26 @@ export const chatBackendCleanupOutboxHandlers = {
11295
const failure = failures.find((error) => error !== undefined)
11396
if (failure) throw failure
11497
},
98+
99+
/**
100+
* Deletes everything under the `chat-images/<chatId>/` prefix of each chat whose row is gone, so
101+
* an image a turn published after the delete read the transcript goes too. Sweeps several chats
102+
* at once, checkpoints how many leading chats are finished, and continues in a later run once
103+
* the deadline nears.
104+
*/
105+
[CHAT_INLINE_IMAGE_CLEANUP_EVENT]: async (rawPayload, context) => {
106+
const { chatIds, swept } = chatInlineImageCleanupPayloadSchema.parse(rawPayload)
107+
context.signal.throwIfAborted()
108+
const pending = chatIds.slice(swept)
109+
const survivingIds = await survivingChatIds(pending)
110+
const outcomes = await mapWithConcurrency(pending, STORAGE_CONCURRENCY, (chatId) =>
111+
survivingIds.has(chatId) ? Promise.resolve('done' as const) : sweepChatImages(chatId, context)
112+
)
113+
const unfinished = outcomes.findIndex((outcome) => outcome !== 'done')
114+
if (unfinished === -1) return
115+
if (unfinished > 0) await context.checkpointPayload({ swept: swept + unfinished })
116+
if (outcomes.includes('continue'))
117+
return continueOutboxHandler('Continuing chat inline image cleanup')
118+
return deferOutboxHandler('Chat inline image cleanup removed nothing from its listing')
119+
},
115120
} satisfies OutboxHandlerRegistry

0 commit comments

Comments
 (0)