Skip to content

Commit 5089d12

Browse files
committed
fix(files): preserve upload history across creator handoff
1 parent 341a721 commit 5089d12

10 files changed

Lines changed: 325 additions & 65 deletions

File tree

‎apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
} from '@/lib/api/contracts/v1/admin'
3434
import { parseRequest } from '@/lib/api/server'
3535
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
36+
import { ProjectConflictError } from '@/lib/projects/membership'
3637
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
3738
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
3839
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
@@ -309,7 +310,10 @@ export const DELETE = withRouteHandler(
309310
workspaceId,
310311
})
311312
} catch (error) {
312-
if (error instanceof WorkspaceBillingAccountRemovalError) {
313+
if (
314+
error instanceof WorkspaceBillingAccountRemovalError ||
315+
error instanceof ProjectConflictError
316+
) {
313317
return badRequestResponse(error.message)
314318
}
315319
logger.error('Admin API: Failed to remove workspace member', { error, workspaceId, memberId })

‎apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
import { parseRequest } from '@/lib/api/server'
5151
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
5252
import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
53+
import { ProjectConflictError } from '@/lib/projects/membership'
5354
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
5455
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
5556
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
@@ -389,10 +390,19 @@ export const DELETE = withRouteHandler(
389390
return notFoundResponse('Workspace member')
390391
}
391392

392-
await db.transaction(async (tx) => {
393-
const result = await revokeWorkspaceAccessTx(tx, { workspaceId, userId: userId })
394-
if (!result.revoked) throw new WorkspaceBillingAccountRemovalError()
393+
const removed = await db.transaction(async (tx) => {
394+
const result = await revokeWorkspaceAccessTx(tx, {
395+
workspaceId,
396+
userId,
397+
expectedPermissionId: existingPermission.id,
398+
})
399+
if (!result.revoked) {
400+
if (result.reason === 'membership-changed') return false
401+
throw new WorkspaceBillingAccountRemovalError()
402+
}
403+
return true
395404
})
405+
if (!removed) return notFoundResponse('Workspace member')
396406

397407
logger.info(`Admin API: Removed user ${userId} from workspace ${workspaceId}`)
398408

@@ -409,7 +419,10 @@ export const DELETE = withRouteHandler(
409419

410420
return singleResponse({ removed: true, userId, workspaceId })
411421
} catch (error) {
412-
if (error instanceof WorkspaceBillingAccountRemovalError) {
422+
if (
423+
error instanceof WorkspaceBillingAccountRemovalError ||
424+
error instanceof ProjectConflictError
425+
) {
413426
return badRequestResponse(error.message)
414427
}
415428
logger.error('Admin API: Failed to remove workspace member', {

‎apps/sim/lib/projects/resource-handoff.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
ProjectConflictError,
1919
tryLockProjects,
2020
} from '@/lib/projects/membership'
21+
import { handoffFileCreatorsInTx } from '@/lib/uploads/contexts/workspace/creator-handoff'
2122

2223
/**
2324
* An org admin, else a teammate who administers every surviving environment. With `hold`,
@@ -140,10 +141,11 @@ export async function handoffProjectCreatorReferencesTx(
140141
'Project successor must administer every surviving environment'
141142
)
142143
}
143-
await tx
144-
.update(workspaceFiles)
145-
.set({ userId: successorId })
146-
.where(and(eq(workspaceFiles.projectId, record.id), eq(workspaceFiles.userId, departingUserId)))
144+
await handoffFileCreatorsInTx(
145+
tx,
146+
and(eq(workspaceFiles.projectId, record.id), eq(workspaceFiles.userId, departingUserId)),
147+
successorId
148+
)
147149
await tx
148150
.update(folder)
149151
.set({ userId: successorId })

‎apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
3333
import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
3434
import { runCli } from '@/lib/mothership/agent-cli/run-cli'
35+
import { handoffFileCreatorsInTx } from '@/lib/uploads/contexts/workspace/creator-handoff'
3536
import {
3637
deleteWorkspaceFileVersion,
3738
fetchWorkspaceFileBuffer,
@@ -133,6 +134,110 @@ describe('workspace file version history in PostgreSQL', () => {
133134
expect(await versionRows(fixture.fileId)).toEqual([])
134135
})
135136

137+
it('keeps original attribution, bytes and revision through repeated creator handoff and a later edit', async () => {
138+
const fixture = await seedFile('original')
139+
const [before] = await db
140+
.select()
141+
.from(workspaceFiles)
142+
.where(eq(workspaceFiles.id, fixture.fileId))
143+
for (const successor of [fixture.bobId, fixture.aliceId, fixture.bobId]) {
144+
await db.transaction((tx) =>
145+
handoffFileCreatorsInTx(tx, eq(workspaceFiles.id, fixture.fileId), successor)
146+
)
147+
}
148+
const [after] = await db
149+
.select()
150+
.from(workspaceFiles)
151+
.where(eq(workspaceFiles.id, fixture.fileId))
152+
expect(after).toMatchObject({
153+
key: before.key,
154+
contentUpdatedAt: before.contentUpdatedAt,
155+
uploadedAt: before.uploadedAt,
156+
sizeBytes: before.sizeBytes,
157+
userId: fixture.bobId,
158+
})
159+
expect(await versionRows(fixture.fileId)).toHaveLength(1)
160+
await updateWorkspaceFileContent(
161+
fixture.workspaceId,
162+
fixture.fileId,
163+
fixture.bobId,
164+
Buffer.from('second'),
165+
undefined,
166+
{ version: { source: 'api', authorUserId: fixture.bobId } }
167+
)
168+
const rows = await versionRows(fixture.fileId)
169+
expect(rows.map((row) => [row.version, row.source, row.authorUserIds])).toEqual([
170+
[1, 'upload', [fixture.aliceId]],
171+
[2, 'api', [fixture.bobId]],
172+
])
173+
expect(await readVersionBytes(fixture.workspaceId, fixture.fileId, rows[0].key)).toBe(
174+
'original'
175+
)
176+
await db.transaction((tx) =>
177+
handoffFileCreatorsInTx(tx, eq(workspaceFiles.id, fixture.fileId), fixture.aliceId)
178+
)
179+
expect(await versionRows(fixture.fileId)).toEqual(rows)
180+
})
181+
182+
it('does not manufacture original authorship for previously overwritten unrecorded content', async () => {
183+
const fixture = await seedFile('overwritten')
184+
await db
185+
.update(workspaceFiles)
186+
.set({ contentUpdatedAt: new Date(Date.now() + 5000) })
187+
.where(eq(workspaceFiles.id, fixture.fileId))
188+
await db.transaction((tx) =>
189+
handoffFileCreatorsInTx(tx, eq(workspaceFiles.id, fixture.fileId), fixture.bobId)
190+
)
191+
expect(await versionRows(fixture.fileId)).toMatchObject([
192+
{ version: 1, source: 'unknown', authorUserIds: [] },
193+
])
194+
})
195+
196+
it('keeps an empty shell unversioned until its first content write after handoff', async () => {
197+
const fixture = await seedFile('')
198+
await db.transaction((tx) =>
199+
handoffFileCreatorsInTx(tx, eq(workspaceFiles.id, fixture.fileId), fixture.bobId)
200+
)
201+
expect(await versionRows(fixture.fileId)).toEqual([])
202+
await updateWorkspaceFileContent(
203+
fixture.workspaceId,
204+
fixture.fileId,
205+
fixture.bobId,
206+
Buffer.from('first'),
207+
undefined,
208+
{ version: { source: 'api', authorUserId: fixture.bobId } }
209+
)
210+
expect(await versionRows(fixture.fileId)).toMatchObject([
211+
{ version: 1, source: 'api', authorUserIds: [fixture.bobId] },
212+
])
213+
})
214+
215+
it('serializes content writes with creator handoff without duplicate or misattributed history', async () => {
216+
const fixture = await seedFile('original')
217+
await Promise.all([
218+
db.transaction((tx) =>
219+
handoffFileCreatorsInTx(tx, eq(workspaceFiles.id, fixture.fileId), fixture.bobId)
220+
),
221+
updateWorkspaceFileContent(
222+
fixture.workspaceId,
223+
fixture.fileId,
224+
fixture.aliceId,
225+
Buffer.from('second'),
226+
undefined,
227+
{ version: { source: 'api', authorUserId: fixture.aliceId } }
228+
),
229+
])
230+
const rows = await versionRows(fixture.fileId)
231+
expect(rows.map((row) => [row.version, row.authorUserIds])).toEqual([
232+
[1, [fixture.aliceId]],
233+
[2, [fixture.aliceId]],
234+
])
235+
expect(rows.filter((row) => row.supersededAt === null)).toHaveLength(1)
236+
expect(await readVersionBytes(fixture.workspaceId, fixture.fileId, rows[0].key)).toBe(
237+
'original'
238+
)
239+
})
240+
136241
it.each([
137242
['named', 'download'],
138243
['anonymous', 'download'],
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { workspaceFiles } from '@sim/db/schema'
2+
import { asc, inArray, type SQL } from 'drizzle-orm'
3+
import type { DbTransaction } from '@/lib/db/types'
4+
import { snapshotWorkspaceFileSecretProvenanceInTx } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
5+
import {
6+
isVersionHeadCurrent,
7+
loadWorkspaceFileVersionHead,
8+
materializeWorkspaceFileVersionInTx,
9+
} from '@/lib/uploads/contexts/workspace/workspace-file-versions'
10+
import { getWorkspaceFileSize } from '@/lib/uploads/shared/types'
11+
12+
/** Freezes implicit history under the writer lock before replacing a shared file's live creator. */
13+
export async function handoffFileCreatorsInTx(
14+
tx: DbTransaction,
15+
condition: SQL | undefined,
16+
successorId: string
17+
) {
18+
if (!condition) throw new Error('File creator handoff requires a scope')
19+
const files = await tx
20+
.select()
21+
.from(workspaceFiles)
22+
.where(condition)
23+
.orderBy(asc(workspaceFiles.id))
24+
.for('update')
25+
for (const file of files) {
26+
if (file.context !== 'workspace' && file.context !== 'project') continue
27+
const head = await loadWorkspaceFileVersionHead(file.id, tx)
28+
if (isVersionHeadCurrent(head, file) || (!head && getWorkspaceFileSize(file) === 0)) continue
29+
const provenance =
30+
file.projectId && file.secretProvenanceVersion === null
31+
? { status: 'unknown' as const, entries: [] }
32+
: await snapshotWorkspaceFileSecretProvenanceInTx(
33+
tx,
34+
file.id,
35+
file.contentUpdatedAt,
36+
file.secretProvenanceVersion
37+
)
38+
await materializeWorkspaceFileVersionInTx(tx, file, head, provenance, new Date())
39+
}
40+
if (!files.length) return
41+
await tx
42+
.update(workspaceFiles)
43+
.set({ userId: successorId, updatedAt: new Date() })
44+
.where(
45+
inArray(
46+
workspaceFiles.id,
47+
files.map((file) => file.id)
48+
)
49+
)
50+
}

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-versions.ts‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,35 @@ interface RecordedWorkspaceFileVersion {
191191
releasedKeys: string[]
192192
}
193193

194+
/** Materializes current bytes under the file lock before content or creator attribution changes. */
195+
export async function materializeWorkspaceFileVersionInTx(
196+
tx: DbTransaction,
197+
previous: WorkspaceFileRow,
198+
head: WorkspaceFileVersionSummaryRow | undefined,
199+
provenance: WorkspaceFileSecretProvenanceSnapshot,
200+
now: Date
201+
): Promise<WorkspaceFileVersionSummaryRow> {
202+
if (isVersionHeadCurrent(head, previous) && head) return head
203+
if (head && head.supersededAt === null) await supersedeVersionInTx(tx, head.id, now)
204+
const original = !head && isOriginalUploadContent(previous)
205+
const [materialized] = await tx
206+
.insert(workspaceFileVersion)
207+
.values({
208+
id: generateId(),
209+
fileId: previous.id,
210+
workspaceId: previous.workspaceId,
211+
version: head ? head.version + 1 : INITIAL_WORKSPACE_FILE_VERSION,
212+
...contentColumns(previous, provenance),
213+
contentHash: null,
214+
source: original ? 'upload' : 'unknown',
215+
authorUserIds: original && previous.userId ? [previous.userId] : [],
216+
createdAt: previous.contentUpdatedAt,
217+
updatedAt: previous.contentUpdatedAt,
218+
})
219+
.returning(versionSummaryColumns)
220+
return materialized
221+
}
222+
194223
/**
195224
* Records a committed content write in the file's history. Runs inside the content-write
196225
* transaction, under the file row's lock, which serializes version numbering and coalescing
@@ -215,24 +244,13 @@ export async function recordWorkspaceFileVersionInTx(
215244
if (!params.previousProvenance) {
216245
throw new Error('Outgoing workspace file content needs a provenance snapshot to be versioned')
217246
}
218-
if (head && head.supersededAt === null) await supersedeVersionInTx(tx, head.id, now)
219-
const original = !head && isOriginalUploadContent(previous)
220-
const [materialized] = await tx
221-
.insert(workspaceFileVersion)
222-
.values({
223-
id: generateId(),
224-
fileId: previous.id,
225-
workspaceId: params.workspaceId,
226-
version: head ? head.version + 1 : INITIAL_WORKSPACE_FILE_VERSION,
227-
...contentColumns(previous, params.previousProvenance),
228-
contentHash: null,
229-
source: original ? 'upload' : 'unknown',
230-
authorUserIds: original && previous.userId ? [previous.userId] : [],
231-
createdAt: previous.contentUpdatedAt,
232-
updatedAt: previous.contentUpdatedAt,
233-
})
234-
.returning(versionSummaryColumns)
235-
head = materialized
247+
head = await materializeWorkspaceFileVersionInTx(
248+
tx,
249+
previous,
250+
head,
251+
params.previousProvenance,
252+
now
253+
)
236254
}
237255

238256
const nextColumns = { ...contentColumns(next, params.nextProvenance), updatedAt: now }

‎apps/sim/lib/workspace-files/search/owners.integration.ts‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -481,31 +481,37 @@ describe('owner-scoped indexed file search on PostgreSQL and stored bytes', () =
481481
}
482482
)
483483

484-
check('keeps empty and recursive folder scopes exact after the creator is detached', async () => {
485-
const f = await fixture()
486-
const folder = await createFolder(f, 'Architecture')
487-
const child = await createFolder(f, 'Backend', folder.folder.id)
488-
const created = await create(f, 'folderneedle', { folderId: child.folder.id })
489-
await db
490-
.update(workspaceFiles)
491-
.set({ userId: null })
492-
.where(eq(workspaceFiles.id, created.file.id))
493-
await indexWorkspaceFileForSearch({ owner: f.owner, ...(await claim(created.file.id)) }, signal)
494-
expect((await search(f, 'folderneedle', { folderPaths: [] })).results).toEqual([])
495-
expect(
496-
(
497-
await search(f, 'folderneedle', {
498-
folderPaths: ['/Architecture'],
499-
includeSubfolders: false,
500-
})
501-
).results
502-
).toEqual([])
503-
expect(
504-
(await search(f, 'folderneedle', { folderPaths: ['/Architecture'] })).results.map(
505-
(hit) => hit.fileId
484+
check(
485+
'keeps empty and recursive folder scopes exact after the creator is reassigned',
486+
async () => {
487+
const f = await fixture()
488+
const folder = await createFolder(f, 'Architecture')
489+
const child = await createFolder(f, 'Backend', folder.folder.id)
490+
const created = await create(f, 'folderneedle', { folderId: child.folder.id })
491+
await db
492+
.update(workspaceFiles)
493+
.set({ userId: f.readerId })
494+
.where(eq(workspaceFiles.id, created.file.id))
495+
await indexWorkspaceFileForSearch(
496+
{ owner: f.owner, ...(await claim(created.file.id)) },
497+
signal
506498
)
507-
).toEqual([created.file.id])
508-
})
499+
expect((await search(f, 'folderneedle', { folderPaths: [] })).results).toEqual([])
500+
expect(
501+
(
502+
await search(f, 'folderneedle', {
503+
folderPaths: ['/Architecture'],
504+
includeSubfolders: false,
505+
})
506+
).results
507+
).toEqual([])
508+
expect(
509+
(await search(f, 'folderneedle', { folderPaths: ['/Architecture'] })).results.map(
510+
(hit) => hit.fileId
511+
)
512+
).toEqual([created.file.id])
513+
}
514+
)
509515

510516
check('rejects forged owner payloads without changing the current revision', async () => {
511517
const f = await fixture()

0 commit comments

Comments
 (0)