refactor(workspaces): apply /simplify review — invert stranded-provisioning to opt-in

Per multi-angle simplify/altitude review of the auto-provisioning fix:

- archiveWorkspace's stranded-member handling is now opt-in
  (provisionFallbackForStrandedMembers, default off) instead of opt-out
  (force). A future caller of archiveWorkspace that doesn't know about
  this feature now gets the safe default (no side-effect workspace
  creation) rather than having to remember to pass force: true. Kept
  the logic inside archiveWorkspace's single transaction rather than
  splitting into two composable functions, since that would either
  reintroduce the TOCTOU race (separate transactions) or require
  threading an external tx through archiveWorkspace (bigger, riskier
  change for uncertain benefit).
- provisionedWorkspaceUserIds is now returned from the transaction
  callback instead of mutated via an outer `let`.
- Deduplicated CreateWorkspaceParams in app/api/workspaces/route.ts —
  now Omit<CreateWorkspaceRecordParams, 'executor'> instead of a
  hand-copied interface that could drift from the shared type.
- Added a comment at the createWorkspaceRecord call site in
  archiveWorkspace noting it intentionally bypasses
  getWorkspaceCreationPolicy (system-provisioned safety net, not user
  self-service).
- Renamed a colliding local test helper (createTx used for two
  different mock shapes in adjacent lib/workspaces test files).

disableUserResources (ban flow) no longer needs a flag at all — the
default now matches its existing behavior.

Author: waleedlatif1

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

@@ -83,6 +83,7 @@ describe('DELETE /api/workspaces/[id]', () => {
     expect(body).toEqual({ success: true })
     expect(mockArchiveWorkspace).toHaveBeenCalledWith('workspace-1', {
       requestId: 'workspace-workspace-1',
+      provisionFallbackForStrandedMembers: true,
       actorId: 'user-admin',
       actorName: 'Admin',
       actorEmail: 'admin@example.com',

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

@@ -250,6 +250,7 @@ export const DELETE = withRouteHandler(
 
       const archiveResult = await archiveWorkspace(workspaceId, {
         requestId: `workspace-${workspaceId}`,
+        provisionFallbackForStrandedMembers: true,
         actorId: session.user.id,
         actorName: session.user.name,
         actorEmail: session.user.email,

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

@@ -12,7 +12,7 @@ import type { PlanCategory } from '@/lib/billing/plan-helpers'
 import { PlatformEvents } from '@/lib/core/telemetry'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { captureServerEvent } from '@/lib/posthog/server'
-import { createWorkspaceRecord } from '@/lib/workspaces/create'
+import { type CreateWorkspaceRecordParams, createWorkspaceRecord } from '@/lib/workspaces/create'
 import {
   CONTACT_OWNER_TO_UPGRADE_REASON,
   evaluateWorkspaceInvitePolicy,
@@ -261,15 +261,7 @@ async function createDefaultWorkspace(
   })
 }
 
-interface CreateWorkspaceParams {
-  userId: string
-  name: string
-  skipDefaultWorkflow?: boolean
-  explicitColor?: string
-  organizationId: string | null
-  workspaceMode: WorkspaceMode
-  billedAccountUserId: string
-}
+type CreateWorkspaceParams = Omit<CreateWorkspaceRecordParams, 'executor'>
 
 async function createWorkspace({
   userId,

apps/sim/lib/workflows/lifecycle.test.ts

@@ -147,7 +147,7 @@ describe('disableUserResources', () => {
     vi.clearAllMocks()
   })
 
-  it('force-archives every owned workspace so banning is never blocked by other members', async () => {
+  it('archives every owned workspace without opting into fallback provisioning, so banning is never blocked by other members', async () => {
     mockSelect.mockReturnValue(createSelectChain([{ id: 'workspace-1' }, { id: 'workspace-2' }]))
     mockDelete.mockReturnValue({ where: vi.fn().mockResolvedValue([]) })
     mockArchiveWorkspace.mockResolvedValue({ archived: true, workspaceName: 'Workspace' })
@@ -157,11 +157,11 @@ describe('disableUserResources', () => {
     expect(mockArchiveWorkspace).toHaveBeenCalledTimes(2)
     expect(mockArchiveWorkspace).toHaveBeenCalledWith(
       'workspace-1',
-      expect.objectContaining({ force: true })
+      expect.not.objectContaining({ provisionFallbackForStrandedMembers: true })
     )
     expect(mockArchiveWorkspace).toHaveBeenCalledWith(
       'workspace-2',
-      expect.objectContaining({ force: true })
+      expect.not.objectContaining({ provisionFallbackForStrandedMembers: true })
     )
     expect(mockDelete).toHaveBeenCalled()
   })

apps/sim/lib/workflows/lifecycle.ts

@@ -360,9 +360,7 @@ export async function disableUserResources(userId: string): Promise<void> {
     .where(and(eq(workspace.ownerId, userId), isNull(workspace.archivedAt)))
 
   await Promise.all([
-    // force: true — a banned user's owned workspaces must be fully disabled regardless of
-    // whether other members would be left with zero workspaces.
-    ...ownedWorkspaces.map((w) => archiveWorkspace(w.id, { requestId, force: true })),
+    ...ownedWorkspaces.map((w) => archiveWorkspace(w.id, { requestId })),
     db.delete(apiKey).where(eq(apiKey.userId, userId)),
   ])
 

apps/sim/lib/workspaces/create.test.ts

@@ -29,7 +29,7 @@ vi.mock('@/lib/workspaces/colors', () => ({
 
 import { createWorkspaceRecord } from './create'
 
-function createTx() {
+function createInsertOnlyTx() {
   return {
     insert: vi.fn().mockImplementation(() => ({
       values: vi.fn().mockResolvedValue([]),
@@ -43,7 +43,7 @@ describe('createWorkspaceRecord', () => {
   })
 
   it('opens its own transaction when no executor is provided', async () => {
-    const tx = createTx()
+    const tx = createInsertOnlyTx()
     mockTransaction.mockImplementation(async (callback: (trx: typeof tx) => Promise<void>) =>
       callback(tx)
     )
@@ -66,7 +66,7 @@ describe('createWorkspaceRecord', () => {
   })
 
   it('runs directly against a provided executor instead of opening a nested transaction', async () => {
-    const tx = createTx()
+    const tx = createInsertOnlyTx()
 
     await createWorkspaceRecord({
       userId: 'user-1',
@@ -82,7 +82,7 @@ describe('createWorkspaceRecord', () => {
   })
 
   it('skips the default workflow insert when skipDefaultWorkflow is set', async () => {
-    const tx = createTx()
+    const tx = createInsertOnlyTx()
 
     await createWorkspaceRecord({
       userId: 'user-1',
@@ -100,7 +100,7 @@ describe('createWorkspaceRecord', () => {
   })
 
   it('adds a second admin permission row for the billed account when it differs from the owner in org mode', async () => {
-    const tx = createTx()
+    const tx = createInsertOnlyTx()
 
     await createWorkspaceRecord({
       userId: 'user-1',

apps/sim/lib/workspaces/lifecycle.test.ts

@@ -105,18 +105,21 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1' })
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
 
     expect(result).toEqual({
       archived: true,
       workspaceName: 'Workspace 1',
     })
     expect(mockArchiveWorkflowsForWorkspace).toHaveBeenCalledWith('workspace-1', {
       requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
     })
     expect(tx.update).toHaveBeenCalledTimes(8)
     expect(tx.delete).toHaveBeenCalledTimes(1)
-    expect(mockListAccessibleWorkspaceRowsForUser).not.toHaveBeenCalled()
     expect(mockCreateWorkspaceRecord).not.toHaveBeenCalled()
     expect(mockTransaction).toHaveBeenCalledWith(expect.any(Function), {
       isolationLevel: 'serializable',
@@ -142,6 +145,7 @@ describe('workspace lifecycle', () => {
 
     const result = await archiveWorkspace('workspace-1', {
       requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
       actorId: 'admin-1',
       actorName: 'Admin',
       actorEmail: 'admin@example.com',
@@ -172,7 +176,6 @@ describe('workspace lifecycle', () => {
         }),
       })
     )
-    // Deletion is never blocked — the workspace is still archived alongside the fallback creation.
     expect(tx.update).toHaveBeenCalledTimes(8)
     expect(tx.delete).toHaveBeenCalledTimes(1)
   })
@@ -194,7 +197,10 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    await archiveWorkspace('workspace-1', { requestId: 'req-1' })
+    await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
 
     expect(mockCreateWorkspaceRecord).toHaveBeenCalled()
     expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
@@ -219,7 +225,10 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1' })
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
 
     expect(result).toEqual({
       archived: true,
@@ -240,8 +249,6 @@ describe('workspace lifecycle', () => {
       archivedAt: null,
     })
     mockArchiveWorkflowsForWorkspace.mockResolvedValue(0)
-    // The org admin's only *explicit* permission row is on workspace-1, but they still have
-    // access to workspace-2 purely through their organization admin role.
     mockListAccessibleWorkspaceRowsForUser.mockResolvedValue([
       accessibleWorkspaceRow('workspace-1'),
       accessibleWorkspaceRow('workspace-2'),
@@ -252,7 +259,10 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1' })
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
 
     expect(result).toEqual({
       archived: true,
@@ -280,18 +290,20 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1' })
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
 
     expect(result).toEqual({
       archived: true,
       workspaceName: 'Workspace 1',
     })
     expect(mockCreateWorkspaceRecord).not.toHaveBeenCalled()
-    // No knowledge bases found, so the two KB-dependent updates (document, knowledgeConnector) are skipped.
     expect(tx.update).toHaveBeenCalledTimes(8)
   })
 
-  it('skips the stranded-member check and provisioning entirely when force is set (ban flow)', async () => {
+  it('never checks or provisions when provisionFallbackForStrandedMembers is not set (ban flow default)', async () => {
     mockGetWorkspaceWithOwner.mockResolvedValue({
       id: 'workspace-1',
       name: 'Workspace 1',
@@ -305,7 +317,7 @@ describe('workspace lifecycle', () => {
       callback(tx)
     )
 
-    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1', force: true })
+    const result = await archiveWorkspace('workspace-1', { requestId: 'req-1' })
 
     expect(result).toEqual({
       archived: true,

apps/sim/lib/workspaces/lifecycle.ts

@@ -34,11 +34,13 @@ const FALLBACK_WORKSPACE_NAME = 'My Workspace'
 interface ArchiveWorkspaceOptions {
   requestId: string
   /**
-   * Skips auto-provisioning replacement workspaces for members who'd otherwise be stranded.
-   * Only for account-disable flows: a banned user's owned workspaces must be fully disabled, and
-   * the banned user specifically should not be handed a fresh workspace as a side effect.
+   * Opts into auto-provisioning a replacement workspace for any member who'd otherwise be left
+   * with zero active workspaces. Off by default so archival stays a pure "delete this workspace"
+   * primitive for callers that don't ask for it (e.g. the account-disable flow, where a banned
+   * user's workspaces must be fully archived without handing the banned user a fresh one).
+   * Only the interactive DELETE route should set this.
    */
-  force?: boolean
+  provisionFallbackForStrandedMembers?: boolean
   /** Attributed as the actor on the audit log entry for any auto-provisioned replacement workspace. */
   actorId?: string
   actorName?: string | null
@@ -112,43 +114,44 @@ export async function archiveWorkspace(
 
   // serializable: without it, two concurrent deletions sharing a sole member could each read a
   // pre-deletion workspace count and both skip provisioning a replacement. Postgres detects this
-  // write skew under serializable isolation and aborts one transaction. Skipped when force is
-  // set, since that path never runs the stranded-member check at all.
-  const transactionConfig = options.force
-    ? undefined
-    : ({ isolationLevel: 'serializable' } as const)
-
-  let provisionedWorkspaceUserIds: string[] = []
-
-  await db.transaction(async (tx) => {
-    if (!options.force) {
-      const strandedUserIds = await findMembersStrandedByArchival(tx, workspaceId)
-      for (const userId of strandedUserIds) {
-        const fallbackWorkspace = await createWorkspaceRecord({
-          userId,
-          name: FALLBACK_WORKSPACE_NAME,
-          organizationId: null,
-          workspaceMode: WORKSPACE_MODE.PERSONAL,
-          billedAccountUserId: userId,
-          executor: tx,
-        })
+  // write skew under serializable isolation and aborts one transaction. Only needed when the
+  // stranded-member check actually runs.
+  const transactionConfig = options.provisionFallbackForStrandedMembers
+    ? ({ isolationLevel: 'serializable' } as const)
+    : undefined
+
+  const provisionedWorkspaceUserIds = await db.transaction(async (tx) => {
+    if (!options.provisionFallbackForStrandedMembers) {
+      return []
+    }
+
+    const strandedUserIds = await findMembersStrandedByArchival(tx, workspaceId)
+    for (const userId of strandedUserIds) {
+      // Intentionally bypasses getWorkspaceCreationPolicy: this is a system-provisioned safety
+      // net (never blocked by "who can create a workspace" rules), not user self-service.
+      const fallbackWorkspace = await createWorkspaceRecord({
+        userId,
+        name: FALLBACK_WORKSPACE_NAME,
+        organizationId: null,
+        workspaceMode: WORKSPACE_MODE.PERSONAL,
+        billedAccountUserId: userId,
+        executor: tx,
+      })
 
-        if (options.actorId) {
-          recordAudit({
-            workspaceId: fallbackWorkspace.id,
-            actorId: options.actorId,
-            actorName: options.actorName,
-            actorEmail: options.actorEmail,
-            action: AuditAction.WORKSPACE_CREATED,
-            resourceType: AuditResourceType.WORKSPACE,
-            resourceId: fallbackWorkspace.id,
-            resourceName: fallbackWorkspace.name,
-            description: `Auto-created replacement workspace "${fallbackWorkspace.name}" for a member left with no workspace after deleting "${workspaceRecord.name}"`,
-            metadata: { deletedWorkspaceId: workspaceId, recipientUserId: userId },
-          })
-        }
+      if (options.actorId) {
+        recordAudit({
+          workspaceId: fallbackWorkspace.id,
+          actorId: options.actorId,
+          actorName: options.actorName,
+          actorEmail: options.actorEmail,
+          action: AuditAction.WORKSPACE_CREATED,
+          resourceType: AuditResourceType.WORKSPACE,
+          resourceId: fallbackWorkspace.id,
+          resourceName: fallbackWorkspace.name,
+          description: `Auto-created replacement workspace "${fallbackWorkspace.name}" for a member left with no workspace after deleting "${workspaceRecord.name}"`,
+          metadata: { deletedWorkspaceId: workspaceId, recipientUserId: userId },
+        })
       }
-      provisionedWorkspaceUserIds = strandedUserIds
     }
 
     await tx
@@ -272,6 +275,8 @@ export async function archiveWorkspace(
         updatedAt: now,
       })
       .where(and(eq(workspace.id, workspaceId), isNull(workspace.archivedAt)))
+
+    return strandedUserIds
   }, transactionConfig)
 
   await archiveWorkflowsForWorkspace(workspaceId, options)