Skip to content

Commit 0b7eed6

Browse files
committed
fix(projects): close lifecycle races and surface rollout conflicts
1 parent 552ce68 commit 0b7eed6

20 files changed

Lines changed: 397 additions & 112 deletions

File tree

‎apps/sim/app/api/folders/[id]/duplicate/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { getSession } from '@/lib/auth'
1313
import { asOrchestrationError } from '@/lib/core/orchestration/types'
1414
import { generateRequestId } from '@/lib/core/utils/request'
1515
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
16-
import type { DbOrTx } from '@/lib/db/types'
16+
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
1717
import { deduplicateFolderName } from '@/lib/folders/naming'
1818
import { nextFolderSortOrder } from '@/lib/folders/orchestration'
1919
import { assertFolderCollectionHasRoom, toFolderApi } from '@/lib/folders/queries'
@@ -409,7 +409,7 @@ async function duplicateFolderStructure(
409409
}
410410

411411
async function duplicateWorkflowsInFolderTree(
412-
tx: DbOrTx,
412+
tx: DbTransaction,
413413
sourceWorkspaceId: string,
414414
targetWorkspaceId: string,
415415
folderMapping: Map<string, string>,

‎apps/sim/app/api/superuser/import-workflow/route.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,17 +136,19 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
136136
null
137137
)
138138

139-
await db.insert(workflow).values(
140-
await buildNewWorkflowRow(db, {
141-
id: newWorkflowId,
142-
userId: session.user.id,
143-
workspaceId: targetWorkspaceId,
144-
folderId: null,
145-
name: dedupedName,
146-
description: sourceWorkflow.description,
147-
variables: sourceWorkflow.variables || {},
148-
})
149-
)
139+
await db.transaction(async (tx) => {
140+
await tx.insert(workflow).values(
141+
await buildNewWorkflowRow(tx, {
142+
id: newWorkflowId,
143+
userId: session.user.id,
144+
workspaceId: targetWorkspaceId,
145+
folderId: null,
146+
name: dedupedName,
147+
description: sourceWorkflow.description,
148+
variables: sourceWorkflow.variables || {},
149+
})
150+
)
151+
})
150152

151153
// Save using existing persistence logic
152154
const saveResult = await saveWorkflowToNormalizedTables(newWorkflowId, importedData, {

‎apps/sim/app/api/v1/admin/workflows/import/route.ts‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,18 @@ export const POST = withRouteHandler(
115115
const workflowId = generateId()
116116
const dedupedName = await deduplicateWorkflowName(workflowName, workspaceId, folderId || null)
117117

118-
await db.insert(workflow).values(
119-
await buildNewWorkflowRow(db, {
120-
id: workflowId,
121-
userId: workspaceData.ownerId,
122-
workspaceId,
123-
folderId: folderId || null,
124-
name: dedupedName,
125-
description: workflowDescription,
126-
})
127-
)
118+
await db.transaction(async (tx) => {
119+
await tx.insert(workflow).values(
120+
await buildNewWorkflowRow(tx, {
121+
id: workflowId,
122+
userId: workspaceData.ownerId,
123+
workspaceId,
124+
folderId: folderId || null,
125+
name: dedupedName,
126+
description: workflowDescription,
127+
})
128+
)
129+
})
128130

129131
/**
130132
* Same normalization the editor and the v1 import API run, via the one

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -350,16 +350,18 @@ async function importSingleWorkflow(
350350
const workflowId = generateId()
351351
const dedupedName = await deduplicateWorkflowName(workflowName, workspaceId, targetFolderId)
352352

353-
await db.insert(workflow).values(
354-
await buildNewWorkflowRow(db, {
355-
id: workflowId,
356-
userId: ownerId,
357-
workspaceId,
358-
folderId: targetFolderId,
359-
name: dedupedName,
360-
description: workflowData.metadata?.description || 'Imported via Admin API',
361-
})
362-
)
353+
await db.transaction(async (tx) => {
354+
await tx.insert(workflow).values(
355+
await buildNewWorkflowRow(tx, {
356+
id: workflowId,
357+
userId: ownerId,
358+
workspaceId,
359+
folderId: targetFolderId,
360+
name: dedupedName,
361+
description: workflowData.metadata?.description || 'Imported via Admin API',
362+
})
363+
)
364+
})
363365

364366
/**
365367
* Same normalization the editor, the v1 import API and the single-workflow

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { deleteWorkspaceBodySchema, updateWorkspaceContract } from '@/lib/api/co
77
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
88
import { getSession } from '@/lib/auth'
99
import { changeWorkspaceStoragePayerInTx } from '@/lib/billing/storage/payer-transfer'
10+
import { OrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
1011
import { captureServerEvent } from '@/lib/posthog/server'
1112
import { archiveWorkspace } from '@/lib/workspaces/lifecycle'
1213

@@ -316,6 +317,12 @@ export const DELETE = withRouteHandler(
316317

317318
return NextResponse.json({ success: true })
318319
} catch (error) {
320+
if (error instanceof OrchestrationError) {
321+
return NextResponse.json(
322+
{ error: error.message },
323+
{ status: statusForOrchestrationError(error.code) }
324+
)
325+
}
319326
logger.error(`Error deleting workspace ${workspaceId}:`, error)
320327
return NextResponse.json({ error: 'Failed to delete workspace' }, { status: 500 })
321328
}

‎apps/sim/ee/access-control/components/project-issue-restrictions.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client'
22

33
import { Checkbox, Chip } from '@sim/emcn'
4+
import { isApiClientError } from '@/lib/api/client/errors'
45
import { useProjects } from '@/hooks/queries/projects'
56

67
interface ProjectIssueRestrictionsProps {
@@ -16,6 +17,9 @@ export function ProjectIssueRestrictions({
1617
onChange,
1718
}: ProjectIssueRestrictionsProps) {
1819
const projects = useProjects(organizationId)
20+
if (projects.isPending || (isApiClientError(projects.error) && projects.error.status === 503)) {
21+
return null
22+
}
1923
const selected = new Set(value)
2024
return (
2125
<div className='flex flex-col gap-2'>
@@ -24,7 +28,6 @@ export function ProjectIssueRestrictions({
2428
For selected Projects, teammates governed by this group need access to every active
2529
environment to use Issues.
2630
</p>
27-
{projects.isPending && <p className='text-small'>Loading Projects…</p>}
2831
{projects.error && (
2932
<p className='text-[var(--text-error)] text-small'>{projects.error.message}</p>
3033
)}

‎apps/sim/ee/workspace-forking/lib/create-fork.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ export async function createFork(params: CreateForkParams): Promise<CreateForkRe
233233
)
234234
}
235235
const parentProject = await requireForkProject(tx, source.id)
236+
await setForkLockTimeout(tx)
236237
if (admission) {
237238
await lockForkRevision(tx, { sourceWorkspaceId: source.id })
238239
await assertForkPreviewFresh(tx, { sourceWorkspaceId: source.id }, admission)

‎apps/sim/hooks/queries/projects.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useInfiniteQuery } from '@tanstack/react-query'
2+
import { isApiClientError } from '@/lib/api/client/errors'
23
import { requestJson } from '@/lib/api/client/request'
34
import { listProjectsContract } from '@/lib/api/contracts/projects'
45

@@ -20,6 +21,8 @@ export function useProjects(organizationId: string) {
2021
}),
2122
getNextPageParam: (page) => page.nextCursor,
2223
staleTime: PROJECT_LIST_STALE_TIME,
24+
retry: (failureCount, error) =>
25+
!(isApiClientError(error) && error.status === 503) && failureCount < 3,
2326
enabled: Boolean(organizationId),
2427
})
2528
}

‎apps/sim/lib/api/contracts/user.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ export const accountDeletionBlockerSchema = z.object({
477477
'shared_workspace',
478478
'organization_workspace',
479479
'data_drain_owner',
480+
'project_lifecycle',
480481
]),
481482
/** A sentence naming both the obstacle and the way out. */
482483
message: z.string(),

‎apps/sim/lib/projects/__integration__/foundation.integration.ts‎

Lines changed: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,13 @@ import {
4242
import { archiveProjectInTransaction } from '@/lib/projects/lifecycle'
4343
import {
4444
createProjectForWorkspace,
45+
lockProject,
4546
lockWorkspaceProject,
47+
splitForkProject,
4648
transferWorkspaceProjects,
4749
} from '@/lib/projects/membership'
48-
import { restoreWorkflow } from '@/lib/workflows/lifecycle'
50+
import { getAccountDeletionPlan } from '@/lib/users/account-deletion'
51+
import { disableUserResources, restoreWorkflow } from '@/lib/workflows/lifecycle'
4952
import { buildNewWorkflowRow } from '@/lib/workflows/persistence/new-workflow-row'
5053
import { createWorkspaceInTransaction } from '@/lib/workspaces/create'
5154
import { archiveWorkspace } from '@/lib/workspaces/lifecycle'
@@ -1083,6 +1086,148 @@ describe('Project foundation at the database and application boundary', () => {
10831086
}
10841087
)
10851088

1089+
check(
1090+
'account deletion preview reports a surviving Project losing its last active environment',
1091+
async () => {
1092+
const f = await fixture(false)
1093+
await db
1094+
.delete(permissions)
1095+
.where(and(eq(permissions.entityId, f.ids[0]), eq(permissions.userId, f.teammateId)))
1096+
await db
1097+
.delete(permissions)
1098+
.where(and(eq(permissions.entityId, f.ids[1]), eq(permissions.userId, f.ownerId)))
1099+
await db.insert(permissions).values({
1100+
id: generateId(),
1101+
entityId: f.ids[1],
1102+
entityType: 'workspace',
1103+
userId: f.teammateId,
1104+
permissionType: 'admin',
1105+
})
1106+
await db
1107+
.update(workspace)
1108+
.set({ archivedAt: new Date(), ownerId: f.teammateId, billedAccountUserId: f.teammateId })
1109+
.where(eq(workspace.id, f.ids[1]))
1110+
const plan = await getAccountDeletionPlan(f.ownerId)
1111+
expect(plan.workspacesToDelete.map((row) => row.id)).toEqual([f.ids[0]])
1112+
expect(plan.blockers).toEqual([
1113+
{ code: 'project_lifecycle', message: expect.stringContaining('Archive') },
1114+
])
1115+
await expect(
1116+
db.transaction((tx) => prepareProjectsForAccountDeletion(tx, f.ownerId, [f.ids[0]]))
1117+
).rejects.toMatchObject({ code: 'conflict' })
1118+
await db.transaction((tx) => archiveProjectInTransaction(tx, f.projectId))
1119+
expect((await getAccountDeletionPlan(f.ownerId)).blockers).toEqual([])
1120+
}
1121+
)
1122+
1123+
check(
1124+
'account teardown can choose an organization admin without per-environment grants',
1125+
async () => {
1126+
const f = await fixture()
1127+
if (!f.organizationId) throw new Error('Missing organization fixture')
1128+
await db.insert(member).values({
1129+
id: generateId(),
1130+
organizationId: f.organizationId,
1131+
userId: f.teammateId,
1132+
role: 'admin',
1133+
createdAt: new Date(),
1134+
})
1135+
await db.transaction((tx) => prepareProjectsForAccountDeletion(tx, f.ownerId, []))
1136+
const [record] = await db.select().from(project).where(eq(project.id, f.projectId))
1137+
expect(record.ownerId).toBe(f.teammateId)
1138+
}
1139+
)
1140+
1141+
check(
1142+
'account teardown re-reads Projects created by an unlink while waiting for the old Project',
1143+
async () => {
1144+
const f = await fixture(false)
1145+
const held = createDeferred<number>()
1146+
const release = createDeferred<void>()
1147+
let detachedProjectId: string | null = null
1148+
const unlink = db.transaction(async (tx) => {
1149+
await lockProject(tx, f.projectId)
1150+
const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
1151+
held.resolve(connection.pid)
1152+
await release.promise
1153+
detachedProjectId = await splitForkProject(tx, f.ids[1])
1154+
await tx
1155+
.update(workspace)
1156+
.set({ forkedFromWorkspaceId: null })
1157+
.where(eq(workspace.id, f.ids[1]))
1158+
})
1159+
const blocker = await held.promise
1160+
const deletion = db.transaction((tx) =>
1161+
prepareProjectsForAccountDeletion(tx, f.ownerId, [f.ids[1]])
1162+
)
1163+
try {
1164+
let waiting = false
1165+
for (let attempt = 0; attempt < 100; attempt++) {
1166+
const rows = await db.execute(
1167+
sql`SELECT 1 FROM pg_stat_activity WHERE ${blocker} = ANY(pg_blocking_pids(pid))`
1168+
)
1169+
if (rows.length) {
1170+
waiting = true
1171+
break
1172+
}
1173+
await sleep(10)
1174+
}
1175+
expect(waiting).toBe(true)
1176+
} finally {
1177+
release.resolve()
1178+
await unlink
1179+
}
1180+
await deletion
1181+
if (!detachedProjectId) throw new Error('Unlink did not create a Project')
1182+
expect(await db.select().from(project).where(eq(project.id, detachedProjectId))).toEqual([])
1183+
}
1184+
)
1185+
1186+
check(
1187+
'banning a former owner cannot archive environments transferred while waiting for the Project',
1188+
async () => {
1189+
const f = await fixture(false)
1190+
const held = createDeferred<number>()
1191+
const release = createDeferred<void>()
1192+
const transfer = db.transaction(async (tx) => {
1193+
await lockProject(tx, f.projectId)
1194+
const [connection] = await tx.execute<{ pid: number }>(sql`SELECT pg_backend_pid() AS pid`)
1195+
held.resolve(connection.pid)
1196+
await release.promise
1197+
await tx
1198+
.update(workspace)
1199+
.set({ ownerId: f.teammateId })
1200+
.where(inArray(workspace.id, f.ids))
1201+
})
1202+
const blocker = await held.promise
1203+
const ban = disableUserResources(f.ownerId)
1204+
try {
1205+
let waiting = false
1206+
for (let attempt = 0; attempt < 100; attempt++) {
1207+
const rows = await db.execute(
1208+
sql`SELECT 1 FROM pg_stat_activity WHERE ${blocker} = ANY(pg_blocking_pids(pid))`
1209+
)
1210+
if (rows.length) {
1211+
waiting = true
1212+
break
1213+
}
1214+
await sleep(10)
1215+
}
1216+
expect(waiting).toBe(true)
1217+
} finally {
1218+
release.resolve()
1219+
await transfer
1220+
}
1221+
await ban
1222+
const rows = await db.select().from(workspace).where(inArray(workspace.id, f.ids))
1223+
expect(rows.every((row) => row.archivedAt === null && row.ownerId === f.teammateId)).toBe(
1224+
true
1225+
)
1226+
const [record] = await db.select().from(project).where(eq(project.id, f.projectId))
1227+
expect(record.archivedAt).toBeNull()
1228+
}
1229+
)
1230+
10861231
check(
10871232
'account teardown transfers surviving Projects and atomically removes wholly private Projects',
10881233
async () => {

0 commit comments

Comments
 (0)