Skip to content

Commit a4eaf4e

Browse files
committed
resolve conflicts
1 parent dd355cb commit a4eaf4e

10 files changed

Lines changed: 16913 additions & 133 deletions

File tree

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

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
88
const {
99
mockTransaction,
1010
mockSelectRows,
11-
mockCheckStorageQuota,
1211
mockFilterForkableChatFiles,
1312
mockListForkableChatFiles,
1413
mockPlanChatFileCopies,
@@ -24,7 +23,6 @@ const {
2423
} = vi.hoisted(() => ({
2524
mockTransaction: vi.fn(),
2625
mockSelectRows: vi.fn(),
27-
mockCheckStorageQuota: vi.fn(),
2826
// Real (pure) cut semantics so tests drive selection through row.messageId:
2927
// rows with a NULL/undefined messageId are kept in every fork.
3028
mockFilterForkableChatFiles: vi.fn(
@@ -89,10 +87,6 @@ vi.mock('@/lib/copilot/resources/persistence', () => ({
8987

9088
vi.mock('@/lib/copilot/request/http', () => copilotHttpMock)
9189

92-
vi.mock('@/lib/billing/storage', () => ({
93-
checkStorageQuota: mockCheckStorageQuota,
94-
}))
95-
9690
vi.mock('@/lib/copilot/chat/fork-chat-files', () => ({
9791
filterForkableChatFiles: mockFilterForkableChatFiles,
9892
listForkableChatFiles: mockListForkableChatFiles,
@@ -218,7 +212,6 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
218212
})
219213
mockSelectRows.mockResolvedValue([parentRow])
220214
mockListForkableChatFiles.mockResolvedValue([])
221-
mockCheckStorageQuota.mockResolvedValue({ allowed: true })
222215
mockLoadCopilotChatMessages.mockResolvedValue(threeMessages)
223216
mockPlanChatFileCopies.mockResolvedValue({
224217
idMap: new Map(),
@@ -316,42 +309,6 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
316309
expect(appended[1].map((m: { id: string }) => m.id)).toEqual(['msg-1', 'msg-2'])
317310
})
318311

319-
it('fails up front with the quota error when copied bytes would exceed the limit', async () => {
320-
mockListForkableChatFiles.mockResolvedValue([
321-
{ size: 600, workspaceId: 'ws-1' },
322-
{ size: 400, workspaceId: 'ws-1' },
323-
])
324-
mockCheckStorageQuota.mockResolvedValue({ allowed: false, error: 'Storage limit exceeded' })
325-
326-
const res = await POST(createRequest('chat-1'), makeContext('chat-1'))
327-
328-
expect(res.status).toBe(400)
329-
expect(mockCheckStorageQuota).toHaveBeenCalledWith('user-1', 1000)
330-
expect(mockTransaction).not.toHaveBeenCalled()
331-
expect(mockExecuteChatFileBlobCopies).not.toHaveBeenCalled()
332-
})
333-
334-
it('excludes uncopyable rows (no workspaceId) from the quota sum', async () => {
335-
// planChatFileCopies skips workspaceId-less legacy rows, so their bytes
336-
// must not count against the gate.
337-
mockListForkableChatFiles.mockResolvedValue([{ size: 600, workspaceId: 'ws-1' }, { size: 400 }])
338-
339-
const res = await POST(createRequest('chat-1'), makeContext('chat-1'))
340-
341-
expect(res.status).toBe(200)
342-
expect(mockCheckStorageQuota).toHaveBeenCalledWith('user-1', 600)
343-
})
344-
345-
it('skips the quota check entirely when no chat-owned rows are in the cut', async () => {
346-
// The chat owns one file, but it was born after the fork point.
347-
mockListForkableChatFiles.mockResolvedValue([
348-
{ id: 'wf_late', size: 500, context: 'mothership', messageId: 'msg-3' },
349-
])
350-
const res = await POST(createRequest('chat-1'), makeContext('chat-1'))
351-
expect(res.status).toBe(200)
352-
expect(mockCheckStorageQuota).not.toHaveBeenCalled()
353-
})
354-
355312
it('forks the chat: copies kept uploads, rewrites references, clones agent state', async () => {
356313
const blobTasks = [
357314
{
@@ -378,17 +335,12 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
378335
expect(body.success).toBe(true)
379336
expect(typeof body.id).toBe('string')
380337

381-
expect(mockCheckStorageQuota).toHaveBeenCalledWith('user-1', 100)
382-
383338
// The real rewriter runs: the kept message's view-URL points at the copy.
384339
const appended = mockAppendCopilotChatMessages.mock.calls[0]
385340
expect(appended[0]).toBe(body.id)
386341
expect(appended[1][0].content).toBe(`See ![cat](/api/files/view/${NEW_FILE_ID})`)
387342

388-
expect(mockExecuteChatFileBlobCopies).toHaveBeenCalledWith(blobTasks, {
389-
userId: 'user-1',
390-
workspaceId: 'ws-1',
391-
})
343+
expect(mockExecuteChatFileBlobCopies).toHaveBeenCalledWith(blobTasks)
392344

393345
const goCall = mockFetchGo.mock.calls[0]
394346
expect(goCall[0]).toBe('http://mothership.test/api/chats/fork')

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

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { eq, inArray } from 'drizzle-orm'
66
import { type NextRequest, NextResponse } from 'next/server'
77
import { forkMothershipChatContract } from '@/lib/api/contracts/mothership-chats'
88
import { parseRequest } from '@/lib/api/server'
9-
import { checkStorageQuota } from '@/lib/billing/storage'
109
import {
1110
executeChatFileBlobCopies,
1211
filterForkableChatFiles,
@@ -49,11 +48,11 @@ const logger = createLogger('ForkChatAPI')
4948
* at-or-before the fork point (a file travels iff the user message that
5049
* carried it is kept). Resources and copilot-side state are copied.
5150
*
52-
* Every copied file gets a fresh row id and storage key, bytes are physically
53-
* copied and counted against the storage quota, and every in-transcript file
54-
* reference is re-pointed at the copies so the new chat survives deletion of
55-
* the source chat. File resources whose chat-owned file was NOT copied
56-
* (uploads born after the cut) are dropped from the new chat's resources
51+
* Every copied file gets a fresh row id and storage key, and every
52+
* in-transcript file reference is re-pointed at the copies so the new chat
53+
* survives deletion of the source chat. Mothership files remain excluded from
54+
* workspace storage accounting. File resources whose chat-owned file was NOT
55+
* copied (uploads born after the cut) are dropped from the new chat's resources
5756
* rather than left as ghosts pointing at the source chat's files.
5857
*/
5958
export const POST = withRouteHandler(
@@ -111,19 +110,6 @@ export const POST = withRouteHandler(
111110
chatOwnedFiles,
112111
new Set(forkedMessages.map((m) => m.id))
113112
)
114-
// Sum only rows the plan will actually copy — planChatFileCopies skips
115-
// rows with no workspaceId, so counting their bytes could reject a fork
116-
// whose real copies fit within quota.
117-
const totalFileBytes = sourceFiles.reduce(
118-
(sum, row) => (row.workspaceId ? sum + row.size : sum),
119-
0
120-
)
121-
if (totalFileBytes > 0) {
122-
const quotaCheck = await checkStorageQuota(userId, totalFileBytes)
123-
if (!quotaCheck.allowed) {
124-
return createBadRequestResponse(quotaCheck.error || 'Storage limit exceeded')
125-
}
126-
}
127113

128114
// Resources are stored as a jsonb array on the chat row. They carry no
129115
// timestamps, so they can't be timeline-cut like messages — instead,
@@ -202,10 +188,7 @@ export const POST = withRouteHandler(
202188
}
203189
const newChat = result.row
204190

205-
const { copied, failed, failedCopyIds } = await executeChatFileBlobCopies(result.blobTasks, {
206-
userId,
207-
workspaceId: parent.workspaceId ?? undefined,
208-
})
191+
const { copied, failed, failedCopyIds } = await executeChatFileBlobCopies(result.blobTasks)
209192
if (failed > 0) {
210193
// A failed blob copy leaves a committed row with no bytes behind it.
211194
// Cleanly absent beats present-but-broken: hard-delete the dead rows

apps/sim/lib/copilot/chat/fork-chat-files.test.ts

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,11 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockGenerateKey, mockDownloadFile, mockUploadFile, mockIncrementStorageUsage } = vi.hoisted(
7-
() => ({
8-
mockGenerateKey: vi.fn(),
9-
mockDownloadFile: vi.fn(),
10-
mockUploadFile: vi.fn(),
11-
mockIncrementStorageUsage: vi.fn(),
12-
})
13-
)
6+
const { mockGenerateKey, mockDownloadFile, mockUploadFile } = vi.hoisted(() => ({
7+
mockGenerateKey: vi.fn(),
8+
mockDownloadFile: vi.fn(),
9+
mockUploadFile: vi.fn(),
10+
}))
1411

1512
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
1613
generateWorkspaceFileKey: mockGenerateKey,
@@ -21,10 +18,6 @@ vi.mock('@/lib/uploads/core/storage-service', () => ({
2118
uploadFile: mockUploadFile,
2219
}))
2320

24-
vi.mock('@/lib/billing/storage', () => ({
25-
incrementStorageUsage: mockIncrementStorageUsage,
26-
}))
27-
2821
import {
2922
executeChatFileBlobCopies,
3023
type ForkableChatFileRow,
@@ -141,14 +134,10 @@ describe('executeChatFileBlobCopies', () => {
141134
vi.clearAllMocks()
142135
mockDownloadFile.mockResolvedValue(Buffer.from('0123456789'))
143136
mockUploadFile.mockResolvedValue(undefined)
144-
mockIncrementStorageUsage.mockResolvedValue(undefined)
145137
})
146138

147-
it('copies bytes to the new key and counts them against the storage quota', async () => {
148-
const result = await executeChatFileBlobCopies([task], {
149-
userId: 'user-1',
150-
workspaceId: 'ws-1',
151-
})
139+
it('copies bytes to the new key without workspace storage accounting', async () => {
140+
const result = await executeChatFileBlobCopies([task])
152141

153142
expect(result).toEqual({ copied: 1, failed: 0, failedCopyIds: [] })
154143
expect(mockUploadFile).toHaveBeenCalledWith(
@@ -159,23 +148,18 @@ describe('executeChatFileBlobCopies', () => {
159148
)
160149
// No `metadata` in the upload call — passing it would insert a second row.
161150
expect(mockUploadFile.mock.calls[0][0].metadata).toBeUndefined()
162-
expect(mockIncrementStorageUsage).toHaveBeenCalledWith('user-1', 10, 'ws-1')
163151
})
164152

165-
it('is best-effort: a failed download skips the file, counts nothing, and reports its copy id', async () => {
153+
it('is best-effort: a failed download skips the file and reports its copy id', async () => {
166154
mockDownloadFile.mockRejectedValueOnce(new Error('blob missing'))
167155

168-
const result = await executeChatFileBlobCopies(
169-
[task, { ...task, copyId: 'wf_copy2', targetKey: 'workspace/ws-1/3-cat.png' }],
170-
{
171-
userId: 'user-1',
172-
workspaceId: 'ws-1',
173-
}
174-
)
156+
const result = await executeChatFileBlobCopies([
157+
task,
158+
{ ...task, copyId: 'wf_copy2', targetKey: 'workspace/ws-1/3-cat.png' },
159+
])
175160

176161
// The first task's download failed — its copy id comes back for row cleanup.
177162
expect(result).toEqual({ copied: 1, failed: 1, failedCopyIds: ['wf_copy'] })
178-
expect(mockIncrementStorageUsage).toHaveBeenCalledTimes(1)
179163
})
180164

181165
it('copies every task even when the batch exceeds the concurrency bound', async () => {
@@ -185,7 +169,7 @@ describe('executeChatFileBlobCopies', () => {
185169
targetKey: `workspace/ws-1/${i}-cat.png`,
186170
}))
187171

188-
const result = await executeChatFileBlobCopies(tasks, { userId: 'user-1' })
172+
const result = await executeChatFileBlobCopies(tasks)
189173

190174
expect(result.copied).toBe(9)
191175
expect(mockUploadFile).toHaveBeenCalledTimes(9)

apps/sim/lib/copilot/chat/fork-chat-files.ts

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { generateShortId } from '@sim/utils/id'
55
import { and, eq, isNull } from 'drizzle-orm'
6-
import { incrementStorageUsage } from '@/lib/billing/storage'
76
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
87
import type { DbOrTx } from '@/lib/db/types'
98
import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
@@ -156,15 +155,13 @@ export async function planChatFileCopies(params: {
156155
* missing) rather than failing the whole fork. Runs a bounded worker pool
157156
* ({@link CHAT_BLOB_COPY_CONCURRENCY}) — media-heavy chats must not pay 2N
158157
* serial storage round-trips, but unbounded fan-out would buffer every file
159-
* in memory at once. Each successfully copied file increments the
160-
* storage-usage counter by its actual byte length. Failed tasks' copy-row ids
161-
* are returned so the caller can delete the dead rows (row exists, blob
162-
* doesn't) instead of leaving them listed in the VFS and resources with
163-
* nothing behind them.
158+
* in memory at once. Mothership files remain excluded from workspace storage
159+
* accounting. Failed tasks' copy-row ids are returned so the caller can delete
160+
* the dead rows (row exists, blob doesn't) instead of leaving them listed in
161+
* the VFS and resources with nothing behind them.
164162
*/
165163
export async function executeChatFileBlobCopies(
166-
blobTasks: ChatBlobCopyTask[],
167-
params: { userId: string; workspaceId?: string }
164+
blobTasks: ChatBlobCopyTask[]
168165
): Promise<{ copied: number; failed: number; failedCopyIds: string[] }> {
169166
let copied = 0
170167
const failedCopyIds: string[] = []
@@ -193,20 +190,6 @@ export async function executeChatFileBlobCopies(
193190
preserveKey: true,
194191
})
195192
copied += 1
196-
try {
197-
// Forked bytes COUNT against the storage quota, deliberately diverging
198-
// from the workspace-fork copy path
199-
// (lib/workspaces/fork/copy/copy-files.ts), which copies blobs without
200-
// counting them. A chat fork stores a second physical copy of every
201-
// kept upload, so the counter must reflect it. Do not "fix" this back
202-
// to the workspace-fork precedent.
203-
await incrementStorageUsage(params.userId, buffer.length, params.workspaceId)
204-
} catch (error) {
205-
logger.error('Failed to update storage tracking for forked chat file', {
206-
targetKey: task.targetKey,
207-
error: getErrorMessage(error),
208-
})
209-
}
210193
} catch (error) {
211194
failedCopyIds.push(task.copyId)
212195
logger.warn('Failed to copy chat file blob during fork', {

packages/db/migrations/0260_unknown_sinister_six.sql

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
ALTER TABLE "paused_executions" ADD COLUMN IF NOT EXISTS "automatic_resume_retry_count" integer DEFAULT 0 NOT NULL;--> statement-breakpoint
22
ALTER TABLE "workspace" ADD COLUMN IF NOT EXISTS "storage_used_bytes" bigint DEFAULT 0 NOT NULL;--> statement-breakpoint
33
ALTER TABLE "workspace" ADD COLUMN IF NOT EXISTS "organization_assigned_at" timestamp;--> statement-breakpoint
4-
ALTER TABLE "workspace_files" ADD COLUMN IF NOT EXISTS "message_id" text;--> statement-breakpoint
54
-- migration-safe: replay-only cleanup of the constraint this same migration adds on the next line; no deployed code knows it yet. A failed concurrent index build below leaves this file unjournaled, and the rerun must not fail on ADD CONSTRAINT already-exists.
65
ALTER TABLE "workspace" DROP CONSTRAINT IF EXISTS "workspace_storage_used_bytes_non_negative";--> statement-breakpoint
76
ALTER TABLE "workspace" ADD CONSTRAINT "workspace_storage_used_bytes_non_negative" CHECK ("workspace"."storage_used_bytes" >= 0) NOT VALID;--> statement-breakpoint
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE "workspace_files" ADD COLUMN "message_id" text;

packages/db/migrations/meta/0260_snapshot.json

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15859,12 +15859,6 @@
1585915859
"primaryKey": false,
1586015860
"notNull": false
1586115861
},
15862-
"message_id": {
15863-
"name": "message_id",
15864-
"type": "text",
15865-
"primaryKey": false,
15866-
"notNull": false
15867-
},
1586815862
"original_name": {
1586915863
"name": "original_name",
1587015864
"type": "text",

0 commit comments

Comments
 (0)