Skip to content

Commit fc33334

Browse files
committed
fix(mothership): order fork publication against a purge of its source
A fork can share keys with its source (organization attachments, files whose copy failed), and chat cleanup deletes a shared key once no remaining chat references it, checking after it deletes the source row. The fork's publish transaction now holds the source row with FOR KEY SHARE: a purge that already removed it refuses the fork (404, worker copy discarded), and one that has not waits for the commit and then sees the fork's references.
1 parent a286f0a commit fc33334

2 files changed

Lines changed: 35 additions & 0 deletions

File tree

‎apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
185185
queueTableRows(copilotChats, [chat])
186186
queueTableRows(copilotChats, [chat])
187187
queueTableRows(member, [{ role: 'member' }])
188+
queueTableRows(copilotChats, [{ id: chat.id }])
188189
const attachments = [
189190
{
190191
id: 'upload-1',
@@ -215,6 +216,26 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
215216
expect(mockAssertActiveWorkspaceAccess).not.toHaveBeenCalled()
216217
})
217218

219+
it('refuses to publish a fork whose source chat was purged while it copied', async () => {
220+
// The fork shares its attachment keys with the source, and cleanup deletes an unreferenced
221+
// key only after deleting the source row: publishing without the source would leave the
222+
// fork pointing at bytes that cleanup is about to delete.
223+
dbChainMockFns.limit.mockReset()
224+
const chat = { ...parentRow, workspaceId: null, organizationId: 'org-1', resources: [] }
225+
queueTableRows(copilotChats, [chat])
226+
queueTableRows(copilotChats, [chat])
227+
queueTableRows(member, [{ role: 'member' }])
228+
queueTableRows(copilotChats, [])
229+
const res = await POST(createRequest('chat-1'), createRouteContext({ chatId: 'chat-1' }))
230+
expect(res.status).toBe(404)
231+
expect(mockAppendCopilotChatMessages).not.toHaveBeenCalled()
232+
expect(mockPublishStatusChanged).not.toHaveBeenCalled()
233+
expect(mockFetchGo.mock.calls.map(([url]) => url)).toEqual([
234+
'http://mothership.test/api/chats/fork',
235+
'http://mothership.test/api/tasks/cleanup',
236+
])
237+
})
238+
218239
it.each(['membership', 'capability'] as const)(
219240
'denies organization forks after %s revocation before copying',
220241
async (revocation) => {

‎apps/sim/lib/mothership/chat/application/fork.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,20 @@ export const forkChat = defineAuthorizedChatUseCase({
156156

157157
/** Publish only after both the file bytes and the worker conversation are prepared. */
158158
await db.transaction(async (tx) => {
159+
/**
160+
* The fork can share keys with its source (organization attachments, files whose copy
161+
* failed), and chat cleanup deletes a shared key once no remaining chat references it,
162+
* checking only after it deletes the source row. Holding the source row until commit
163+
* orders the two: a purge that already removed it refuses this fork, and one that has
164+
* not waits for this commit and then sees the fork's references.
165+
*/
166+
const [source] = await tx
167+
.select({ id: copilotChats.id })
168+
.from(copilotChats)
169+
.where(eq(copilotChats.id, chatId))
170+
.for('key share')
171+
.limit(1)
172+
if (!source) throw new OrchestrationError('not_found', 'Chat not found')
159173
const [row] = await tx
160174
.insert(copilotChats)
161175
.values({

0 commit comments

Comments
 (0)