Skip to content

Commit 19b8d53

Browse files
authored
fix(cleanup): keep shared chat files, sweep inline images by prefix, and make room refetches wait (#9433)
Release-review fixes for #9419/#9427: room callbacks must return their invalidations (tables and workflows rooms now wait for their refetches); the deployed-state hint is set only by active reads; chat cleanup keeps any chat-scoped key a surviving chat in its scope still attaches (one query per scope) and collects a forked workspace upload once no file row names it; inline images are swept by their chat prefix through their own outbox event; malformed attachment keys are skipped. Legacy queued payloads are still accepted.
1 parent 70724d0 commit 19b8d53

27 files changed

Lines changed: 1230 additions & 404 deletions

‎apps/sim/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room.test.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const roots: Root[] = []
2929
function mount(
3030
workspaceId: string,
3131
roomType: RoomType,
32-
onChanged: () => unknown = () => {},
32+
onChanged: () => Promise<unknown> = async () => {},
3333
dedupeKey?: string
3434
): Root {
3535
function Probe() {
@@ -119,7 +119,7 @@ describe('useWorkspaceInvalidationRoom', () => {
119119
})
120120

121121
it('delivers a burst of broadcasts once at once and once after it, not once per broadcast', async () => {
122-
const onChanged = vi.fn()
122+
const onChanged = vi.fn(async () => {})
123123
mount('ws-1', 'workspace-files', onChanged)
124124

125125
for (let index = 0; index < 20; index++) broadcast('workspace-files-changed', 'ws-1')

‎apps/sim/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ interface JoinErrorPayload {
3636
}
3737

3838
/**
39-
* Runs when the room's resources changed. A returned promise is awaited, up to
40-
* {@link DELIVERY_SETTLE_CAP_MS}, before the next delivery starts.
39+
* Runs when the room's resources changed and returns the invalidations it started. The promise is
40+
* awaited, up to {@link DELIVERY_SETTLE_CAP_MS}, before the next delivery starts, so a callback
41+
* that drops it lets the next broadcast cancel and restart refetches still in flight.
4142
*/
42-
type RoomChangedCallback = () => unknown
43+
type RoomChangedCallback = () => Promise<unknown>
4344

4445
interface SharedRoomSubscription {
4546
callbacks: Map<string | symbol, Set<RoomChangedCallback>>
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
5+
import { act } from 'react'
6+
import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const { mockUseSocket } = vi.hoisted(() => ({ mockUseSocket: vi.fn() }))
11+
12+
vi.mock('@/app/workspace/providers/socket-provider', () => ({
13+
useSocket: () => mockUseSocket(),
14+
}))
15+
16+
import { INVALIDATION_COALESCE_MS } from '@/app/workspace/[workspaceId]/hooks/use-workspace-invalidation-room'
17+
import { useWorkspaceTablesRoom } from '@/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room'
18+
import { tableKeys } from '@/hooks/queries/utils/table-keys'
19+
20+
function fakeSocket() {
21+
return { connected: true, emit: vi.fn(), on: vi.fn(), off: vi.fn() }
22+
}
23+
24+
let socket: ReturnType<typeof fakeSocket>
25+
let root: Root | null = null
26+
27+
function broadcast(workspaceId: string) {
28+
for (const [name, handler] of socket.on.mock.calls) {
29+
if (name === 'workspace-tables-changed') handler({ workspaceId })
30+
}
31+
}
32+
33+
describe('useWorkspaceTablesRoom', () => {
34+
beforeEach(() => {
35+
vi.useFakeTimers()
36+
socket = fakeSocket()
37+
mockUseSocket.mockImplementation(() => ({ socket }))
38+
})
39+
40+
afterEach(() => {
41+
if (root) act(() => root?.unmount())
42+
root = null
43+
vi.useRealTimers()
44+
})
45+
46+
it('does not restart a list refetch still in flight when the next broadcast arrives', async () => {
47+
const pending: Array<() => void> = []
48+
const fetchList = vi.fn(
49+
() =>
50+
new Promise<string[]>((resolve) => {
51+
pending.push(() => resolve([]))
52+
})
53+
)
54+
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
55+
queryClient.setQueryData(tableKeys.list('ws-1'), [])
56+
57+
function Probe() {
58+
useQuery({
59+
queryKey: tableKeys.list('ws-1'),
60+
queryFn: fetchList,
61+
staleTime: Number.POSITIVE_INFINITY,
62+
})
63+
useWorkspaceTablesRoom('ws-1')
64+
return null
65+
}
66+
const container = document.createElement('div')
67+
document.body.appendChild(container)
68+
root = createRoot(container)
69+
act(() =>
70+
root?.render(
71+
<QueryClientProvider client={queryClient}>
72+
<Probe />
73+
</QueryClientProvider>
74+
)
75+
)
76+
77+
broadcast('ws-1')
78+
await vi.advanceTimersByTimeAsync(0)
79+
expect(fetchList).toHaveBeenCalledTimes(1)
80+
81+
broadcast('ws-1')
82+
await vi.advanceTimersByTimeAsync(5 * INVALIDATION_COALESCE_MS)
83+
expect(fetchList).toHaveBeenCalledTimes(1)
84+
85+
for (const resolve of pending.splice(0)) resolve()
86+
await vi.advanceTimersByTimeAsync(INVALIDATION_COALESCE_MS)
87+
expect(fetchList).toHaveBeenCalledTimes(2)
88+
})
89+
})

‎apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import { tableKeys } from '@/hooks/queries/utils/table-keys'
1616
*/
1717
export function useWorkspaceTablesRoom(workspaceId: string): void {
1818
const queryClient = useQueryClient()
19-
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () => {
20-
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
21-
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') })
22-
})
19+
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () =>
20+
Promise.all([
21+
queryClient.invalidateQueries({ queryKey: tableKeys.lists() }),
22+
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') }),
23+
])
24+
)
2325
}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-resource-folders.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export function useResourceFolders(
7777
useWorkspaceInvalidationRoom(
7878
isFileResource ? (workspaceId ?? '') : '',
7979
ROOM_TYPES.WORKSPACE_FILES,
80-
() => (workspaceId ? invalidateWorkspaceFileBrowsers(queryClient, workspaceId) : undefined),
80+
() =>
81+
workspaceId ? invalidateWorkspaceFileBrowsers(queryClient, workspaceId) : Promise.resolve(),
8182
WORKSPACE_FILE_BROWSER_INVALIDATION_KEY
8283
)
8384

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-workflows-room.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ import { invalidateWorkflowLists } from '@/hooks/queries/utils/invalidate-workfl
2121
*/
2222
export function useWorkspaceWorkflowsRoom(workspaceId: string): void {
2323
const queryClient = useQueryClient()
24-
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_WORKFLOWS, () => {
25-
invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived'])
26-
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'active') })
27-
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'archived') })
28-
})
24+
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_WORKFLOWS, () =>
25+
Promise.all([
26+
invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived']),
27+
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'active') }),
28+
queryClient.invalidateQueries({ queryKey: folderKeys.list(workspaceId, 'archived') }),
29+
])
30+
)
2931
}

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +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 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.
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.
2520
*/
2621
export async function enqueueAdminRemovedChatCleanup(user: { id: string }): Promise<void> {
2722
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: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
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 several chats at once, never more than ten', async () => {
44+
queueTableRows(copilotChats, [])
45+
let inFlight = 0
46+
let peak = 0
47+
mockSweepStoragePrefix.mockImplementation(async () => {
48+
peak = Math.max(peak, ++inFlight)
49+
await new Promise((resolve) => setImmediate(resolve))
50+
inFlight--
51+
return 'done'
52+
})
53+
54+
await sweepImages({ chatIds }, outboxContext())
55+
56+
expect(peak).toBe(10)
57+
expect(new Set(sweptPrefixes())).toEqual(new Set(chatIds.map(prefixOf)))
58+
})
59+
60+
it('checkpoints the chats finished before the lowest unfinished one and resumes after them', async () => {
61+
queueTableRows(copilotChats, [])
62+
const slow = chatIds[3]
63+
mockSweepStoragePrefix.mockImplementation(async (prefix: string) => {
64+
if (prefix !== prefixOf(slow)) return 'done'
65+
// Finishes after the chats behind it, so only "lowest unfinished" can yield 3 here.
66+
await new Promise((resolve) => setImmediate(resolve))
67+
deadline.near = true
68+
return 'continue'
69+
})
70+
const context = outboxContext()
71+
72+
const result = await sweepImages({ chatIds: chatIds.slice(0, 6) }, context)
73+
74+
expect(result).toMatchObject({ outcome: 'deferred', consumeAttempt: false })
75+
expect(context.checkpointPayload).toHaveBeenCalledWith({ swept: 3 })
76+
77+
queueTableRows(copilotChats, [])
78+
deadline.near = false
79+
mockSweepStoragePrefix.mockReset().mockResolvedValue('done')
80+
await expect(
81+
sweepImages({ chatIds: chatIds.slice(0, 6), swept: 3 }, outboxContext())
82+
).resolves.toBe(undefined)
83+
expect(sweptPrefixes().sort()).toEqual(chatIds.slice(3, 6).map(prefixOf))
84+
})
85+
86+
it('skips a chat id no image key can hold instead of failing the event on every retry', async () => {
87+
queueTableRows(copilotChats, [])
88+
89+
await expect(
90+
sweepImages({ chatIds: ['../escape', chatIds[0]] }, outboxContext())
91+
).resolves.toBe(undefined)
92+
expect(sweptPrefixes()).toEqual([prefixOf(chatIds[0])])
93+
})
94+
95+
it('spends an attempt when a chat’s listing stops advancing', async () => {
96+
queueTableRows(copilotChats, [])
97+
mockSweepStoragePrefix.mockImplementation(async (prefix: string) =>
98+
prefix === prefixOf(chatIds[0]) ? 'stalled' : 'done'
99+
)
100+
const context = outboxContext()
101+
102+
const result = await sweepImages({ chatIds: chatIds.slice(0, 2) }, context)
103+
104+
expect(result).toMatchObject({ outcome: 'deferred' })
105+
expect(result).not.toMatchObject({ consumeAttempt: false })
106+
expect(context.checkpointPayload).not.toHaveBeenCalled()
107+
})
108+
109+
it('spends an attempt when the lowest unfinished chat stalls, though a later one continues', async () => {
110+
queueTableRows(copilotChats, [])
111+
mockSweepStoragePrefix.mockImplementation(async (prefix: string) => {
112+
if (prefix === prefixOf(chatIds[0])) return 'stalled'
113+
deadline.near = true
114+
return 'continue'
115+
})
116+
const context = outboxContext()
117+
118+
const result = await sweepImages({ chatIds: chatIds.slice(0, 2) }, context)
119+
120+
expect(result).toMatchObject({ outcome: 'deferred' })
121+
expect(result).not.toMatchObject({ consumeAttempt: false })
122+
expect(context.checkpointPayload).not.toHaveBeenCalled()
123+
})
124+
})

0 commit comments

Comments
 (0)