fix(workspaces): defer telemetry until commit, avoid second pooled connection mid-transaction

createWorkspaceRecord now only fires workspaceCreated telemetry when it commits
its own transaction; callers passing a nested executor (the fallback-provisioning
path) fire it themselves after their outer transaction commits, avoiding a phantom
event for a workspace that gets rolled back. getActivelyBannedUserIds now accepts
an executor so the stranded-member check runs the banned-user lookup against the
open transaction instead of borrowing a second pooled connection while the first
sits idle-in-transaction.

Author: waleedlatif1

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

@@ -9,7 +9,6 @@ import { createWorkspaceContract } from '@/lib/api/contracts/workspaces'
 import { parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 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 { type CreateWorkspaceRecordParams, createWorkspaceRecord } from '@/lib/workspaces/create'
@@ -282,16 +281,6 @@ async function createWorkspace({
     billedAccountUserId,
   })
 
-  try {
-    PlatformEvents.workspaceCreated({
-      workspaceId: record.id,
-      userId,
-      name,
-    })
-  } catch {
-    // Telemetry should not fail the operation
-  }
-
   const invitePolicy = await getWorkspaceInvitePolicy({
     organizationId,
     workspaceMode,

apps/sim/lib/auth/ban.ts

@@ -1,6 +1,7 @@
 import { db, user } from '@sim/db'
 import { inArray, sql } from 'drizzle-orm'
 import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control'
+import type { DbOrTx } from '@/lib/db/types'
 
 /**
  * True when a ban is currently in effect. Mirrors better-auth admin-plugin
@@ -34,14 +35,22 @@ export async function isEmailBlocked(email: string | null | undefined): Promise<
  * active account ban, or an email/domain in the appconfig blocked lists.
  * One user query plus the cached access-control fetch. Throws on db
  * failure — callers must fail closed.
+ *
+ * Accepts an optional executor so callers already inside a transaction (e.g. a
+ * workspace-archival safety check under `serializable` isolation) can run this
+ * against `tx` instead of borrowing a second pooled connection while the first
+ * sits idle-in-transaction.
  */
-export async function getActivelyBannedUserIds(userIds: string[]): Promise<string[]> {
+export async function getActivelyBannedUserIds(
+  userIds: string[],
+  executor: DbOrTx = db
+): Promise<string[]> {
   const ids = [...new Set(userIds.filter(Boolean))]
   if (ids.length === 0) return []
 
   const [accessControl, rows] = await Promise.all([
     getAccessControlConfig(),
-    db
+    executor
       .select({ id: user.id, email: user.email, banned: user.banned, banExpires: user.banExpires })
       .from(user)
       .where(inArray(user.id, ids)),

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

@@ -3,17 +3,23 @@
  */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockTransaction, mockSaveWorkflowToNormalizedTables } = vi.hoisted(() => ({
-  mockTransaction: vi.fn(),
-  mockSaveWorkflowToNormalizedTables: vi.fn(),
-}))
+const { mockTransaction, mockSaveWorkflowToNormalizedTables, mockWorkspaceCreatedEvent } =
+  vi.hoisted(() => ({
+    mockTransaction: vi.fn(),
+    mockSaveWorkflowToNormalizedTables: vi.fn(),
+    mockWorkspaceCreatedEvent: vi.fn(),
+  }))
 
 vi.mock('@sim/db', () => ({
   db: {
     transaction: mockTransaction,
   },
 }))
 
+vi.mock('@/lib/core/telemetry', () => ({
+  PlatformEvents: { workspaceCreated: mockWorkspaceCreatedEvent },
+}))
+
 vi.mock('@/lib/workflows/defaults', () => ({
   buildDefaultWorkflowArtifacts: () => ({ workflowState: { blocks: {}, edges: [] } }),
 }))
@@ -62,6 +68,12 @@ describe('createWorkspaceRecord', () => {
     expect(record.workspaceMode).toBe('personal')
     expect(tx.insert).toHaveBeenCalledTimes(3)
     expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledTimes(1)
+    // Safe to fire immediately: this call committed its own transaction before returning.
+    expect(mockWorkspaceCreatedEvent).toHaveBeenCalledWith({
+      workspaceId: record.id,
+      userId: 'user-1',
+      name: 'My Workspace',
+    })
   })
 
   it('runs directly against a provided executor instead of opening a nested transaction', async () => {
@@ -78,6 +90,10 @@ describe('createWorkspaceRecord', () => {
 
     expect(mockTransaction).not.toHaveBeenCalled()
     expect(tx.insert).toHaveBeenCalledTimes(3)
+    // Must NOT fire here: the caller's outer transaction hasn't committed yet and could still
+    // roll back, which would make this a phantom event for a workspace that never existed. The
+    // caller owns firing this once its own transaction commits.
+    expect(mockWorkspaceCreatedEvent).not.toHaveBeenCalled()
   })
 
   it('skips the default workflow insert when skipDefaultWorkflow is set', async () => {

apps/sim/lib/workspaces/create.ts

@@ -2,6 +2,7 @@ import { db } from '@sim/db'
 import { permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { generateId } from '@sim/utils/id'
+import { PlatformEvents } from '@/lib/core/telemetry'
 import type { DbOrTx } from '@/lib/db/types'
 import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
 import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
@@ -44,6 +45,10 @@ export interface CreatedWorkspaceRecord {
  * (unless skipped) a default starter workflow. Shared by the `POST /api/workspaces` route and
  * the workspace-archival safety net that auto-provisions a replacement workspace for a member
  * who would otherwise be left with zero workspaces.
+ *
+ * Fires the `workspaceCreated` telemetry event itself only when it manages its own transaction
+ * (no `executor` passed). Callers that pass `executor` are joining an outer transaction that can
+ * still roll back after this returns, so they own firing that event once their transaction commits.
  */
 export async function createWorkspaceRecord({
   userId,
@@ -142,6 +147,18 @@ export async function createWorkspaceRecord({
     throw error
   }
 
+  // Only fire when this call committed its own transaction. When `executor` is a caller-supplied
+  // `tx`, the insert isn't durable yet — the caller's transaction can still roll back after this
+  // function returns, so firing here would risk a phantom event for a workspace that never
+  // existed. Callers that pass `executor` must fire this themselves once their transaction commits.
+  if (!executor) {
+    try {
+      PlatformEvents.workspaceCreated({ workspaceId, userId, name })
+    } catch {
+      // Telemetry should not fail the operation
+    }
+  }
+
   return {
     id: workspaceId,
     name,

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

@@ -11,13 +11,15 @@ const {
   mockListAccessibleWorkspaceRowsForUser,
   mockCreateWorkspaceRecord,
   mockGetActivelyBannedUserIds,
+  mockWorkspaceCreatedEvent,
 } = vi.hoisted(() => ({
   mockSelect: vi.fn(),
   mockTransaction: vi.fn(),
   mockArchiveWorkflowsForWorkspace: vi.fn(),
   mockListAccessibleWorkspaceRowsForUser: vi.fn(),
   mockCreateWorkspaceRecord: vi.fn(),
   mockGetActivelyBannedUserIds: vi.fn(),
+  mockWorkspaceCreatedEvent: vi.fn(),
 }))
 
 const mockGetWorkspaceWithOwner = permissionsMockFns.mockGetWorkspaceWithOwner
@@ -47,6 +49,10 @@ vi.mock('@/lib/auth/ban', () => ({
   getActivelyBannedUserIds: mockGetActivelyBannedUserIds,
 }))
 
+vi.mock('@/lib/core/telemetry', () => ({
+  PlatformEvents: { workspaceCreated: mockWorkspaceCreatedEvent },
+}))
+
 vi.mock('@sim/audit', () => auditMock)
 
 import { archiveWorkspace } from './lifecycle'
@@ -192,6 +198,11 @@ describe('workspace lifecycle', () => {
         }),
       })
     )
+    expect(mockWorkspaceCreatedEvent).toHaveBeenCalledWith({
+      workspaceId: 'fallback-workspace',
+      userId: 'user-victim',
+      name: 'My Workspace',
+    })
     expect(tx.update).toHaveBeenCalledTimes(8)
     expect(tx.delete).toHaveBeenCalledTimes(1)
   })
@@ -247,10 +258,12 @@ describe('workspace lifecycle', () => {
       })
     ).rejects.toThrow('serialization_failure')
 
-    // recordAudit must only ever be called after the transaction has committed — otherwise a
-    // failed transaction (e.g. a serialization abort) would leave a phantom audit entry pointing
-    // at a fallback workspace that was rolled back.
+    // recordAudit and the workspaceCreated telemetry event must only ever fire after the
+    // transaction has committed — otherwise a failed transaction (e.g. a serialization abort)
+    // would leave a phantom audit entry / event pointing at a fallback workspace that was
+    // rolled back.
     expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
+    expect(mockWorkspaceCreatedEvent).not.toHaveBeenCalled()
   })
 
   it('only provisions a fallback for the one member who would actually be stranded', async () => {

apps/sim/lib/workspaces/lifecycle.ts

@@ -20,6 +20,7 @@ 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 { PlatformEvents } from '@/lib/core/telemetry'
 import type { DbOrTx } from '@/lib/db/types'
 import { mcpPubSub } from '@/lib/mcp/pubsub'
 import { mcpService } from '@/lib/mcp/service'
@@ -112,7 +113,7 @@ async function findMembersStrandedByArchival(
     return []
   }
 
-  const bannedUserIds = new Set(await getActivelyBannedUserIds(strandedUserIds))
+  const bannedUserIds = new Set(await getActivelyBannedUserIds(strandedUserIds, executor))
   return strandedUserIds.filter((userId) => !bannedUserIds.has(userId))
 }
 
@@ -294,11 +295,23 @@ export async function archiveWorkspace(
     return fallbacks
   }, transactionConfig)
 
-  // Recorded only after the transaction commits — recordAudit is fire-and-forget and doesn't
-  // participate in the transaction, so recording it earlier could leave a phantom audit entry
-  // pointing at a fallback workspace that got rolled back (e.g. on a serialization failure).
-  if (options.actorId) {
-    for (const fallback of provisionedFallbacks) {
+  // Recorded/fired only after the transaction commits — recordAudit and the telemetry event are
+  // fire-and-forget and don't participate in the transaction, so triggering them earlier could
+  // leave a phantom audit entry / event pointing at a fallback workspace that got rolled back
+  // (e.g. on a serialization failure). `createWorkspaceRecord` defers its own `workspaceCreated`
+  // event for exactly this reason when given an `executor` — this is where it gets fired instead.
+  for (const fallback of provisionedFallbacks) {
+    try {
+      PlatformEvents.workspaceCreated({
+        workspaceId: fallback.workspaceId,
+        userId: fallback.userId,
+        name: fallback.name,
+      })
+    } catch {
+      // Telemetry should not fail the operation
+    }
+
+    if (options.actorId) {
       recordAudit({
         workspaceId: fallback.workspaceId,
         actorId: options.actorId,