improvement(admin): update defaults for better UX (#6112)

* improvement(admin): uupdate defaults for better UX

* refactor: address review nits on the admin/invitation lock work

Correct the attach lock-order comment. The order matches admin move, and what
makes it mandatory is invitation acceptance: it holds `workspace-invitations:<id>`
while waiting for the workspace row, so row-locking first (as this did)
deadlocks against it. The previous ownership-transfer justification did not
hold — that path takes the organization lock before its workspace rows too, so
the two agree on order rather than inverting.

Drop `cancelInvitation`. The `revokeInvitationAsAdmin` extraction left it with
no callers, and an unlocked, unauthorized `status = 'cancelled'` flip sitting
next to the fenced replacement is easy to reach for by mistake.

Drop the unused `executor` parameters from `hasWorkspaceAdminAccess` and
`isOrganizationAdminOrOwner`. No caller threads a transaction through either,
and the former goes back to delegating to `checkWorkspaceAccess` instead of
re-deriving the same permission itself.

Import `chunkArray` from `@sim/utils/helpers` everywhere and remove the
re-export from `batch-delete.ts`, so the symbol has one source rather than a
non-barrel shim plus the package.

Restore the bounded attachability check in `addDashboardOrganizationMember`:
scope the query to the selected ids instead of listing every attachable
workspace and scanning that array per selection.

Resolve the credential-creation permission through
`getEffectiveWorkspacePermission` rather than a second copy of the org-admin
derivation ladder, so the rule cannot drift from the shared resolver.

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor(invitations): drop dead code left by the revocation extraction

`revokeInvitationWorkspaceGrant` lost its only caller when the DELETE route
moved to `revokeInvitationAsAdmin`, leaving a locked wrapper nothing invoked.
Remove it and fold its documentation into `revokeInvitationWorkspaceGrantTx`,
which direct grants and scoped revocation still call. The grant-revocation test
now drives the transactional form directly, so the sibling-grant and
final-grant-cancels behaviour it covers stays under test.

`isSameOrgMember` has had no caller since before this branch — direct grant
resolves membership through `getUserOrganization` inside its own transaction —
so it and its tests go too.

`getWorkspaceMembership` is no longer imported outside its module now that
credential creation reads `getCredentialCreationWorkspaceContext`; make it
module-private rather than leave it on the public surface.

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: remove four uncalled billing and large-value helpers

Each was checked by hand across every file type, including barrel re-exports
and string references, rather than taken from a static analyzer.

`isUserMemberOfOrganization` has no reference anywhere.

`reapplyPaidOrgJoinBillingForExistingMember` only ever ran from two
lock-ordering tests. The transaction-enlisted form it delegated to is what the
subscription webhooks call and what those tests actually assert on, so they now
drive it directly. The assertions are unchanged: the wrapper contributed a
transaction, an organization lock and a membership existence check, none of
which appear in the recorded operations.

`replaceLargeValueReferences` and `replaceLargeValueReferencesWithClient` are
both thin wrappers over `replaceLargeValueReferenceKeysWithClient`, which
execution logging, human-in-the-loop resume and the trace backfill all still
call. The single test covering a wrapper now composes the key collection itself
and targets that live helper.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: icecrasher321

apps/sim/app/api/credentials/route.test.ts

@@ -3,17 +3,24 @@
  *
  * @vitest-environment node
  */
-import { auditMock, authMockFns, createMockRequest, posthogServerMock } from '@sim/testing'
+import {
+  auditMock,
+  authMockFns,
+  createMockRequest,
+  dbChainMockFns,
+  posthogServerMock,
+  resetDbChainMock,
+} from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { TokenServiceAccountValidationError } from '@/lib/credentials/token-service-accounts/errors'
 
 const {
   mockCheckWorkspaceAccess,
-  mockGetWorkspaceMembership,
+  mockGetCredentialCreationWorkspaceContext,
   mockVerifyAndBuildServiceAccountSecret,
 } = vi.hoisted(() => ({
   mockCheckWorkspaceAccess: vi.fn(),
-  mockGetWorkspaceMembership: vi.fn(),
+  mockGetCredentialCreationWorkspaceContext: vi.fn(),
   mockVerifyAndBuildServiceAccountSecret: vi.fn(),
 }))
 
@@ -25,7 +32,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
 }))
 
 vi.mock('@/lib/credentials/environment', () => ({
-  getWorkspaceMembership: mockGetWorkspaceMembership,
+  getCredentialCreationWorkspaceContext: mockGetCredentialCreationWorkspaceContext,
 }))
 
 vi.mock('@/lib/credentials/oauth', () => ({
@@ -52,6 +59,7 @@ const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555'
 describe('POST /api/credentials', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    resetDbChainMock()
     authMockFns.mockGetSession.mockResolvedValue({
       user: { id: 'user-1', name: 'Test User', email: 'test@example.com' },
     })
@@ -60,7 +68,12 @@ describe('POST /api/credentials', () => {
       canWrite: true,
       canAdmin: true,
     })
-    mockGetWorkspaceMembership.mockResolvedValue({ ownerId: 'user-1', memberUserIds: ['user-1'] })
+    mockGetCredentialCreationWorkspaceContext.mockResolvedValue({
+      ownerId: 'user-1',
+      organizationId: 'org-1',
+      memberUserIds: ['user-1'],
+      canWrite: true,
+    })
   })
 
   describe('client-credential service accounts', () => {
@@ -156,5 +169,42 @@ describe('POST /api/credentials', () => {
       expect(data.error).toContain('clientSecret is required')
       expect(mockVerifyAndBuildServiceAccountSecret).not.toHaveBeenCalled()
     })
+
+    it('re-authorizes a personal credential after the shared org/user locks', async () => {
+      mockGetCredentialCreationWorkspaceContext
+        .mockResolvedValueOnce({
+          ownerId: 'user-1',
+          organizationId: 'org-1',
+          memberUserIds: ['user-1'],
+          canWrite: true,
+        })
+        .mockResolvedValueOnce({
+          ownerId: 'org-owner',
+          organizationId: 'org-1',
+          memberUserIds: ['org-owner'],
+          canWrite: false,
+        })
+
+      const req = createMockRequest('POST', {
+        workspaceId: WORKSPACE_ID,
+        type: 'env_personal',
+        envKey: 'MY_API_KEY',
+      })
+
+      const response = await POST(req)
+      const data = await response.json()
+
+      expect(response.status).toBe(403)
+      expect(data).toEqual({ error: 'Write permission required' })
+      expect(mockGetCredentialCreationWorkspaceContext).toHaveBeenCalledTimes(2)
+      expect(dbChainMockFns.execute).toHaveBeenCalled()
+      expect(mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[0]).toBeLessThan(
+        dbChainMockFns.execute.mock.invocationCallOrder[0]
+      )
+      expect(dbChainMockFns.execute.mock.invocationCallOrder.at(-1)).toBeLessThan(
+        mockGetCredentialCreationWorkspaceContext.mock.invocationCallOrder[1]
+      )
+      expect(dbChainMockFns.insert).not.toHaveBeenCalled()
+    })
   })
 })

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

@@ -13,6 +13,7 @@ import {
 } from '@/lib/api/contracts/credentials'
 import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
+import { acquireOrganizationUserMutationLocks } from '@/lib/billing/organizations/membership'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
@@ -21,7 +22,7 @@ import {
   SHARED_CREDENTIAL_TYPES,
 } from '@/lib/credentials/access'
 import { AtlassianValidationError } from '@/lib/credentials/atlassian-service-account'
-import { getWorkspaceMembership } from '@/lib/credentials/environment'
+import { getCredentialCreationWorkspaceContext } from '@/lib/credentials/environment'
 import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
 import {
   ServiceAccountSecretError,
@@ -509,10 +510,52 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       resolvedProviderId === SLACK_CUSTOM_BOT_PROVIDER_ID && clientCredentialId
         ? clientCredentialId
         : generateId()
-    const { ownerId: workspaceOwnerId, memberUserIds: workspaceMemberUserIds } =
-      await getWorkspaceMembership(workspaceId)
 
-    await db.transaction(async (tx) => {
+    const creationResult = await db.transaction(async (tx) => {
+      /**
+       * Discover the organization lock scope inside this transaction, then
+       * acquire the same organization → user → membership locks as org
+       * removal/transfer and re-authorize from the transaction before writing.
+       *
+       * If this insert wins, transfer sees the new source-owned personal
+       * credential and blocks. If transfer wins, its permission/member cleanup
+       * is visible to the authoritative re-read below and the insert is
+       * refused.
+       */
+      const plannedContext = await getCredentialCreationWorkspaceContext({
+        executor: tx,
+        workspaceId,
+        userId: session.user.id,
+      })
+      if (!plannedContext) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+
+      await acquireOrganizationUserMutationLocks(tx, {
+        userId: session.user.id,
+        organizationIds: plannedContext.organizationId ? [plannedContext.organizationId] : [],
+      })
+
+      const currentContext = await getCredentialCreationWorkspaceContext({
+        executor: tx,
+        workspaceId,
+        userId: session.user.id,
+        forUpdate: true,
+      })
+      if (!currentContext) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+      if (currentContext.organizationId !== plannedContext.organizationId) {
+        return {
+          success: false as const,
+          status: 409 as const,
+          error: 'Workspace organization changed while creating the credential. Please retry.',
+        }
+      }
+      if (!currentContext.canWrite) {
+        return { success: false as const, status: 403 as const, error: 'Write permission required' }
+      }
+
       // service_account has no DB-level unique index on (workspaceId, providerId,
       // displayName), so we re-check inside the tx. OAuth/env_* are guarded by
       // partial unique indexes and fall through to the 23505 handler below.
@@ -542,9 +585,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
         updatedAt: now,
       })
 
-      if ((type === 'env_workspace' || type === 'service_account') && workspaceOwnerId) {
-        if (workspaceMemberUserIds.length > 0) {
-          for (const memberUserId of workspaceMemberUserIds) {
+      if ((type === 'env_workspace' || type === 'service_account') && currentContext.ownerId) {
+        if (currentContext.memberUserIds.length > 0) {
+          for (const memberUserId of currentContext.memberUserIds) {
             const isAdmin = memberUserId === session.user.id
             await tx.insert(credentialMember).values({
               id: generateId(),
@@ -572,7 +615,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
           updatedAt: now,
         })
       }
+
+      return { success: true as const }
     })
+    if (!creationResult.success) {
+      return NextResponse.json({ error: creationResult.error }, { status: creationResult.status })
+    }
 
     const [created] = await db
       .select()