fix(workspaces): exclude banned users and include org-admin candidates in stranding check

Two gaps found by Cursor review on the latest commit:

- findMembersStrandedByArchival only considered users with an explicit
  permissions row on the workspace being deleted. An org admin who
  accesses that workspace purely through their organization role (no
  permission row at all) was never evaluated as a stranding candidate,
  so they could be left with zero workspaces without getting a
  fallback. Now unions explicit permission holders with the
  organization's admins/owners (via the member table + ORG_ADMIN_ROLES)
  before running the same accessibility check on every candidate.

- A banned user who still holds a permissions row on someone else's
  workspace (they don't have to own it) would get a fresh fallback
  workspace if stranded by that workspace's deletion — even though
  banned users should never receive new resources. Now filters
  actively-banned userIds (via the existing getActivelyBannedUserIds
  helper, which also covers blocked emails/domains) out of the
  stranded list before provisioning.

Added tests for both: an org admin with no explicit permission row
gets provisioned when stranded, and an actively banned stranded member
does not get provisioned (and gets no audit entry).

Author: waleedlatif1

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

@@ -10,12 +10,14 @@ const {
   mockArchiveWorkflowsForWorkspace,
   mockListAccessibleWorkspaceRowsForUser,
   mockCreateWorkspaceRecord,
+  mockGetActivelyBannedUserIds,
 } = vi.hoisted(() => ({
   mockSelect: vi.fn(),
   mockTransaction: vi.fn(),
   mockArchiveWorkflowsForWorkspace: vi.fn(),
   mockListAccessibleWorkspaceRowsForUser: vi.fn(),
   mockCreateWorkspaceRecord: vi.fn(),
+  mockGetActivelyBannedUserIds: vi.fn(),
 }))
 
 const mockGetWorkspaceWithOwner = permissionsMockFns.mockGetWorkspaceWithOwner
@@ -41,6 +43,10 @@ vi.mock('@/lib/workspaces/create', () => ({
   createWorkspaceRecord: mockCreateWorkspaceRecord,
 }))
 
+vi.mock('@/lib/auth/ban', () => ({
+  getActivelyBannedUserIds: mockGetActivelyBannedUserIds,
+}))
+
 vi.mock('@sim/audit', () => auditMock)
 
 import { archiveWorkspace } from './lifecycle'
@@ -65,9 +71,18 @@ function accessibleWorkspaceRow(workspaceId: string) {
   return { workspace: { id: workspaceId }, permissionType: 'admin' as const }
 }
 
-function createTx(members: Array<{ userId: string }>) {
+function createTx(
+  members: Array<{ userId: string }>,
+  orgAdminMembers: Array<{ userId: string }> = []
+) {
+  const selectDistinct = vi.fn()
+  // First call is always the explicit-permissions query; the second (only reached when the
+  // workspace has an organizationId) is the org-admin query.
+  selectDistinct.mockReturnValueOnce(createMembersChain(members))
+  selectDistinct.mockReturnValueOnce(createMembersChain(orgAdminMembers))
+
   return {
-    selectDistinct: vi.fn().mockReturnValue(createMembersChain(members)),
+    selectDistinct,
     select: vi.fn().mockReturnValue({
       from: vi.fn().mockReturnValue({
         where: vi.fn().mockResolvedValue([]),
@@ -89,6 +104,7 @@ describe('workspace lifecycle', () => {
       }),
     })
     mockCreateWorkspaceRecord.mockResolvedValue({ id: 'fallback-workspace', name: 'My Workspace' })
+    mockGetActivelyBannedUserIds.mockResolvedValue([])
   })
 
   it('archives workspace and dependent resources under serializable isolation', async () => {
@@ -303,6 +319,74 @@ describe('workspace lifecycle', () => {
     expect(tx.update).toHaveBeenCalledTimes(8)
   })
 
+  it('provisions a fallback for an org admin who is stranded but has no explicit permission row', async () => {
+    mockGetWorkspaceWithOwner.mockResolvedValue({
+      id: 'workspace-1',
+      name: 'Workspace 1',
+      ownerId: 'user-1',
+      organizationId: 'org-1',
+      archivedAt: null,
+    })
+    mockArchiveWorkflowsForWorkspace.mockResolvedValue(0)
+    // The org admin has no row in `permissions` for this workspace at all — they only appear as
+    // an org-admin candidate. Their only accessible workspace is this one, so they're stranded.
+    mockListAccessibleWorkspaceRowsForUser.mockResolvedValue([
+      accessibleWorkspaceRow('workspace-1'),
+    ])
+
+    const tx = createTx([], [{ userId: 'user-org-admin-no-row' }])
+    mockTransaction.mockImplementation(async (callback: (trx: typeof tx) => Promise<void>) =>
+      callback(tx)
+    )
+
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+    })
+
+    expect(result).toEqual({
+      archived: true,
+      workspaceName: 'Workspace 1',
+      provisionedWorkspaceUserIds: ['user-org-admin-no-row'],
+    })
+    expect(tx.selectDistinct).toHaveBeenCalledTimes(2)
+    expect(mockCreateWorkspaceRecord).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: 'user-org-admin-no-row' })
+    )
+  })
+
+  it('does not provision a fallback for an actively banned stranded member', async () => {
+    mockGetWorkspaceWithOwner.mockResolvedValue({
+      id: 'workspace-1',
+      name: 'Workspace 1',
+      ownerId: 'user-1',
+      archivedAt: null,
+    })
+    mockArchiveWorkflowsForWorkspace.mockResolvedValue(0)
+    mockListAccessibleWorkspaceRowsForUser.mockResolvedValue([
+      accessibleWorkspaceRow('workspace-1'),
+    ])
+    mockGetActivelyBannedUserIds.mockResolvedValue(['user-banned'])
+
+    const tx = createTx([{ userId: 'user-banned' }])
+    mockTransaction.mockImplementation(async (callback: (trx: typeof tx) => Promise<void>) =>
+      callback(tx)
+    )
+
+    const result = await archiveWorkspace('workspace-1', {
+      requestId: 'req-1',
+      provisionFallbackForStrandedMembers: true,
+      actorId: 'admin-1',
+    })
+
+    expect(result).toEqual({
+      archived: true,
+      workspaceName: 'Workspace 1',
+    })
+    expect(mockCreateWorkspaceRecord).not.toHaveBeenCalled()
+    expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
+  })
+
   it('proceeds without provisioning when every member has another active workspace', async () => {
     mockGetWorkspaceWithOwner.mockResolvedValue({
       id: 'workspace-1',

apps/sim/lib/workspaces/lifecycle.ts

@@ -8,6 +8,7 @@ import {
   knowledgeBase,
   knowledgeConnector,
   mcpServers,
+  member,
   permissions,
   userTableDefinitions,
   workflowMcpServer,
@@ -16,7 +17,9 @@ import {
   workspaceFiles,
 } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
+import { ORG_ADMIN_ROLES } from '@sim/platform-authz/workspace'
 import { and, eq, inArray, isNull, sql } from 'drizzle-orm'
+import { getActivelyBannedUserIds } from '@/lib/auth/ban'
 import type { DbOrTx } from '@/lib/db/types'
 import { mcpPubSub } from '@/lib/mcp/pubsub'
 import { mcpService } from '@/lib/mcp/service'
@@ -55,39 +58,62 @@ interface ArchiveWorkspaceResult {
 }
 
 /**
- * Returns the userIds of explicit workspace members for whom `workspaceId` is their only
- * accessible active (non-archived) workspace. "Accessible" includes workspaces granted through
- * an explicit permission row AND workspaces derived from organization owner/admin role — an org
- * admin can always fall back to the rest of the organization's workspaces even without an
- * explicit permission row on any of them, so they are never stranded by this deletion.
+ * Returns the userIds who would be left with zero accessible active (non-archived) workspaces if
+ * `workspaceId` were archived. Candidates are the union of explicit workspace members AND the
+ * organization's admins/owners — an org admin can access a workspace purely through their org
+ * role with no permission row at all, so they must be checked even though they never show up as
+ * an explicit member. "Accessible" (via `listAccessibleWorkspaceRowsForUser`) already accounts for
+ * that same org-admin-derived access when deciding whether a candidate has another workspace to
+ * fall back to. Actively banned users are excluded from the result — they should never receive a
+ * new resource as a side effect of someone else's action.
  *
  * Must be called against the same executor used to perform the archival, under
  * `serializable` isolation, so the check and the write are atomic with respect to a concurrent
  * deletion of another workspace shared by the same member.
  */
 async function findMembersStrandedByArchival(
   executor: DbOrTx,
-  workspaceId: string
+  workspaceId: string,
+  organizationId: string | null
 ): Promise<string[]> {
-  const members = await executor
+  const explicitMembers = await executor
     .selectDistinct({ userId: permissions.userId })
     .from(permissions)
     .where(and(eq(permissions.entityId, workspaceId), eq(permissions.entityType, 'workspace')))
 
-  if (members.length === 0) {
+  const candidateUserIds = new Set(explicitMembers.map((row) => row.userId))
+
+  if (organizationId) {
+    const orgAdmins = await executor
+      .selectDistinct({ userId: member.userId })
+      .from(member)
+      .where(
+        and(eq(member.organizationId, organizationId), inArray(member.role, [...ORG_ADMIN_ROLES]))
+      )
+    for (const { userId } of orgAdmins) {
+      candidateUserIds.add(userId)
+    }
+  }
+
+  if (candidateUserIds.size === 0) {
     return []
   }
 
   const strandedUserIds: string[] = []
-  for (const { userId } of members) {
+  for (const userId of candidateUserIds) {
     const accessible = await listAccessibleWorkspaceRowsForUser(userId, 'active', executor)
     const hasOtherWorkspace = accessible.some((row) => row.workspace.id !== workspaceId)
     if (!hasOtherWorkspace) {
       strandedUserIds.push(userId)
     }
   }
 
-  return strandedUserIds
+  if (strandedUserIds.length === 0) {
+    return []
+  }
+
+  const bannedUserIds = new Set(await getActivelyBannedUserIds(strandedUserIds))
+  return strandedUserIds.filter((userId) => !bannedUserIds.has(userId))
 }
 
 export async function archiveWorkspace(
@@ -123,7 +149,11 @@ export async function archiveWorkspace(
     const fallbacks: Array<{ userId: string; workspaceId: string; name: string }> = []
 
     if (options.provisionFallbackForStrandedMembers) {
-      const strandedUserIds = await findMembersStrandedByArchival(tx, workspaceId)
+      const strandedUserIds = await findMembersStrandedByArchival(
+        tx,
+        workspaceId,
+        workspaceRecord.organizationId
+      )
       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.