Skip to content

Commit fd2da3d

Browse files
committed
fix(workspaces): auto-provision a replacement workspace instead of blocking deletion
Blocking deletion when it would strand a member was the wrong UX: an org admin doing legitimate cleanup would be stopped because some unrelated, possibly-inactive member happened to have this as their only workspace. archiveWorkspace now always allows the deletion and instead auto-provisions a personal fallback workspace, atomically in the same transaction, for any member who would otherwise be left with zero active workspaces. This guarantees the invariant holds regardless of which route/link/client the affected member's next request comes through, unlike the pre-existing client-side "no workspaces" recovery which only covers the generic /workspace redirect page. Extracted the workspace-creation write (insert workspace + owner permission row + default workflow) out of the POST /api/workspaces route into a shared lib/workspaces/create.ts so both the route and the archival safety net use the same logic, parameterized by an optional executor so it can run inside an existing transaction. The ban flow (disableUserResources) keeps force: true, now meaning "skip stranded-member handling entirely" — a banned user's owned workspaces must still be fully disabled, and the banned user specifically shouldn't be handed a fresh workspace as a side effect.
1 parent b075b9f commit fd2da3d

7 files changed

Lines changed: 492 additions & 265 deletions

File tree

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

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -61,22 +61,6 @@ describe('DELETE /api/workspaces/[id]', () => {
6161
expect(mockArchiveWorkspace).not.toHaveBeenCalled()
6262
})
6363

64-
it('returns 400 and does not record an audit entry when archival would strand a member', async () => {
65-
mockArchiveWorkspace.mockResolvedValue({
66-
archived: false,
67-
workspaceName: 'Victim Workspace',
68-
strandedUserIds: ['user-victim'],
69-
})
70-
71-
const response = await callDelete()
72-
const body = await response.json()
73-
74-
expect(response.status).toBe(400)
75-
expect(body.error).toMatch(/no other workspace/i)
76-
expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
77-
expect(posthogServerMockFns.mockCaptureServerEvent).not.toHaveBeenCalled()
78-
})
79-
8064
it('returns 404 when the workspace does not exist', async () => {
8165
mockArchiveWorkspace.mockResolvedValue({ archived: false })
8266

@@ -115,6 +99,27 @@ describe('DELETE /api/workspaces/[id]', () => {
11599
)
116100
})
117101

102+
it('succeeds and records which members were auto-provisioned a replacement workspace', async () => {
103+
mockArchiveWorkspace.mockResolvedValue({
104+
archived: true,
105+
workspaceName: 'Test Workspace',
106+
provisionedWorkspaceUserIds: ['user-victim'],
107+
})
108+
109+
const response = await callDelete('workspace-1')
110+
const body = await response.json()
111+
112+
expect(response.status).toBe(200)
113+
expect(body).toEqual({ success: true })
114+
expect(auditMockFns.mockRecordAudit).toHaveBeenCalledWith(
115+
expect.objectContaining({
116+
metadata: expect.objectContaining({
117+
provisionedWorkspaceUserIds: ['user-victim'],
118+
}),
119+
})
120+
)
121+
})
122+
118123
it('returns 500 when archival throws unexpectedly', async () => {
119124
mockArchiveWorkspace.mockRejectedValue(new Error('db exploded'))
120125

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,16 +252,6 @@ export const DELETE = withRouteHandler(
252252
requestId: `workspace-${workspaceId}`,
253253
})
254254

255-
if (archiveResult.strandedUserIds?.length) {
256-
return NextResponse.json(
257-
{
258-
error:
259-
'Cannot delete this workspace: one or more members have no other workspace to fall back to',
260-
},
261-
{ status: 400 }
262-
)
263-
}
264-
265255
if (!archiveResult.archived) {
266256
return NextResponse.json({ error: 'Workspace not found' }, { status: 404 })
267257
}
@@ -281,6 +271,9 @@ export const DELETE = withRouteHandler(
281271
workflows: workflowIds.length,
282272
},
283273
archived: archiveResult.archived,
274+
...(archiveResult.provisionedWorkspaceUserIds?.length && {
275+
provisionedWorkspaceUserIds: archiveResult.provisionedWorkspaceUserIds,
276+
}),
284277
},
285278
request,
286279
})

apps/sim/app/api/workspaces/route.ts

Lines changed: 13 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { permissions, settings, type WorkspaceMode, workflow, workspace } from '@sim/db/schema'
3+
import { settings, type WorkspaceMode, workflow } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
5-
import { generateId } from '@sim/utils/id'
65
import { and, eq, isNull } from 'drizzle-orm'
76
import { type NextRequest, NextResponse } from 'next/server'
87
import { listWorkspacesQuerySchema } from '@/lib/api/contracts'
@@ -13,9 +12,7 @@ import type { PlanCategory } from '@/lib/billing/plan-helpers'
1312
import { PlatformEvents } from '@/lib/core/telemetry'
1413
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1514
import { captureServerEvent } from '@/lib/posthog/server'
16-
import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
17-
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
18-
import { getRandomWorkspaceColor } from '@/lib/workspaces/colors'
15+
import { createWorkspaceRecord } from '@/lib/workspaces/create'
1916
import {
2017
CONTACT_OWNER_TO_UPGRADE_REASON,
2118
evaluateWorkspaceInvitePolicy,
@@ -283,90 +280,19 @@ async function createWorkspace({
283280
workspaceMode,
284281
billedAccountUserId,
285282
}: CreateWorkspaceParams) {
286-
const workspaceId = generateId()
287-
const workflowId = generateId()
288-
const now = new Date()
289-
const color = explicitColor || getRandomWorkspaceColor()
290-
291-
try {
292-
await db.transaction(async (tx) => {
293-
await tx.insert(workspace).values({
294-
id: workspaceId,
295-
name,
296-
color,
297-
ownerId: userId,
298-
organizationId,
299-
workspaceMode,
300-
billedAccountUserId,
301-
allowPersonalApiKeys: true,
302-
createdAt: now,
303-
updatedAt: now,
304-
})
305-
306-
const permissionRows = [
307-
{
308-
id: generateId(),
309-
entityType: 'workspace' as const,
310-
entityId: workspaceId,
311-
userId,
312-
permissionType: 'admin' as const,
313-
createdAt: now,
314-
updatedAt: now,
315-
},
316-
]
317-
318-
if (
319-
workspaceMode === WORKSPACE_MODE.ORGANIZATION &&
320-
billedAccountUserId &&
321-
billedAccountUserId !== userId
322-
) {
323-
permissionRows.push({
324-
id: generateId(),
325-
entityType: 'workspace' as const,
326-
entityId: workspaceId,
327-
userId: billedAccountUserId,
328-
permissionType: 'admin' as const,
329-
createdAt: now,
330-
updatedAt: now,
331-
})
332-
}
333-
334-
await tx.insert(permissions).values(permissionRows)
335-
336-
if (!skipDefaultWorkflow) {
337-
await tx.insert(workflow).values({
338-
id: workflowId,
339-
userId,
340-
workspaceId,
341-
folderId: null,
342-
name: 'default-agent',
343-
description: 'Your first workflow - start building here!',
344-
lastSynced: now,
345-
createdAt: now,
346-
updatedAt: now,
347-
isDeployed: false,
348-
runCount: 0,
349-
variables: {},
350-
})
351-
352-
const { workflowState } = buildDefaultWorkflowArtifacts()
353-
await saveWorkflowToNormalizedTables(workflowId, workflowState, tx)
354-
}
355-
356-
logger.info(
357-
skipDefaultWorkflow
358-
? `Created ${workspaceMode} workspace ${workspaceId} for user ${userId}`
359-
: `Created ${workspaceMode} workspace ${workspaceId} with initial workflow ${workflowId} for user ${userId}`
360-
)
361-
})
362-
} catch (error) {
363-
logger.error(`Failed to create workspace ${workspaceId}:`, error)
364-
throw error
365-
}
283+
const record = await createWorkspaceRecord({
284+
userId,
285+
name,
286+
skipDefaultWorkflow,
287+
explicitColor,
288+
organizationId,
289+
workspaceMode,
290+
billedAccountUserId,
291+
})
366292

367293
try {
368294
PlatformEvents.workspaceCreated({
369-
workspaceId,
295+
workspaceId: record.id,
370296
userId,
371297
name,
372298
})
@@ -389,16 +315,7 @@ async function createWorkspace({
389315
: CONTACT_OWNER_TO_UPGRADE_REASON
390316

391317
return {
392-
id: workspaceId,
393-
name,
394-
color,
395-
ownerId: userId,
396-
organizationId,
397-
workspaceMode,
398-
billedAccountUserId,
399-
allowPersonalApiKeys: true,
400-
createdAt: now,
401-
updatedAt: now,
318+
...record,
402319
role: 'owner',
403320
permissions: 'admin',
404321
inviteMembersEnabled: invitePolicy.allowed,
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockTransaction, mockSaveWorkflowToNormalizedTables } = vi.hoisted(() => ({
7+
mockTransaction: vi.fn(),
8+
mockSaveWorkflowToNormalizedTables: vi.fn(),
9+
}))
10+
11+
vi.mock('@sim/db', () => ({
12+
db: {
13+
transaction: mockTransaction,
14+
},
15+
}))
16+
17+
vi.mock('@/lib/workflows/defaults', () => ({
18+
buildDefaultWorkflowArtifacts: () => ({ workflowState: { blocks: {}, edges: [] } }),
19+
}))
20+
21+
vi.mock('@/lib/workflows/persistence/utils', () => ({
22+
saveWorkflowToNormalizedTables: (...args: unknown[]) =>
23+
mockSaveWorkflowToNormalizedTables(...args),
24+
}))
25+
26+
vi.mock('@/lib/workspaces/colors', () => ({
27+
getRandomWorkspaceColor: () => '#123456',
28+
}))
29+
30+
import { createWorkspaceRecord } from './create'
31+
32+
function createTx() {
33+
return {
34+
insert: vi.fn().mockImplementation(() => ({
35+
values: vi.fn().mockResolvedValue([]),
36+
})),
37+
}
38+
}
39+
40+
describe('createWorkspaceRecord', () => {
41+
beforeEach(() => {
42+
vi.clearAllMocks()
43+
})
44+
45+
it('opens its own transaction when no executor is provided', async () => {
46+
const tx = createTx()
47+
mockTransaction.mockImplementation(async (callback: (trx: typeof tx) => Promise<void>) =>
48+
callback(tx)
49+
)
50+
51+
const record = await createWorkspaceRecord({
52+
userId: 'user-1',
53+
name: 'My Workspace',
54+
organizationId: null,
55+
workspaceMode: 'personal',
56+
billedAccountUserId: 'user-1',
57+
})
58+
59+
expect(mockTransaction).toHaveBeenCalledTimes(1)
60+
expect(record.name).toBe('My Workspace')
61+
expect(record.ownerId).toBe('user-1')
62+
expect(record.workspaceMode).toBe('personal')
63+
// workspace insert, permission insert, workflow insert (default workflow not skipped)
64+
expect(tx.insert).toHaveBeenCalledTimes(3)
65+
expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledTimes(1)
66+
})
67+
68+
it('runs directly against a provided executor instead of opening a nested transaction', async () => {
69+
const tx = createTx()
70+
71+
await createWorkspaceRecord({
72+
userId: 'user-1',
73+
name: 'My Workspace',
74+
organizationId: null,
75+
workspaceMode: 'personal',
76+
billedAccountUserId: 'user-1',
77+
executor: tx as never,
78+
})
79+
80+
expect(mockTransaction).not.toHaveBeenCalled()
81+
expect(tx.insert).toHaveBeenCalledTimes(3)
82+
})
83+
84+
it('skips the default workflow insert when skipDefaultWorkflow is set', async () => {
85+
const tx = createTx()
86+
87+
await createWorkspaceRecord({
88+
userId: 'user-1',
89+
name: 'My Workspace',
90+
organizationId: null,
91+
workspaceMode: 'personal',
92+
billedAccountUserId: 'user-1',
93+
skipDefaultWorkflow: true,
94+
executor: tx as never,
95+
})
96+
97+
// workspace insert, permission insert — no workflow insert
98+
expect(tx.insert).toHaveBeenCalledTimes(2)
99+
expect(mockSaveWorkflowToNormalizedTables).not.toHaveBeenCalled()
100+
})
101+
102+
it('adds a second admin permission row for the billed account when it differs from the owner in org mode', async () => {
103+
const tx = createTx()
104+
105+
await createWorkspaceRecord({
106+
userId: 'user-1',
107+
name: 'Org Workspace',
108+
organizationId: 'org-1',
109+
workspaceMode: 'organization',
110+
billedAccountUserId: 'user-billing',
111+
executor: tx as never,
112+
})
113+
114+
const permissionValuesCall = tx.insert.mock.results[1].value.values as ReturnType<typeof vi.fn>
115+
const insertedPermissionRows = permissionValuesCall.mock.calls[0][0]
116+
expect(insertedPermissionRows).toHaveLength(2)
117+
expect(insertedPermissionRows.map((row: { userId: string }) => row.userId)).toEqual([
118+
'user-1',
119+
'user-billing',
120+
])
121+
})
122+
})

0 commit comments

Comments
 (0)