@@ -290,18 +290,27 @@ export const POST = withRouteHandler(
       /** A returned id we did not mint means the conflict branch ran. */
       const wasCreated = written?.id === permissionId
 
-      logger.info(`Admin API: Added user ${userId} to workspace ${workspaceId}`, {
-        permissions: permissionLevel,
-        permissionId,
-      })
+      logger.info(
+        wasCreated
+          ? `Admin API: Added user ${userId} to workspace ${workspaceId}`
+          : `Admin API: Updated user ${userId} permissions in workspace ${workspaceId}`,
+        { permissions: permissionLevel, permissionId }
+      )
 
+      /**
+       * The conflict branch amended a membership that already existed, so it is
+       * a role change rather than an addition — recording it as `MEMBER_ADDED`
+       * would put a join that never happened in the workspace's audit trail.
+       */
       recordAudit({
         workspaceId,
         actorId: 'admin-api',
-        action: AuditAction.MEMBER_ADDED,
+        action: wasCreated ? AuditAction.MEMBER_ADDED : AuditAction.MEMBER_ROLE_CHANGED,
         resourceType: AuditResourceType.WORKSPACE,
         resourceId: workspaceId,
-        description: `Admin API added member to workspace with ${permissionLevel} permissions`,
+        description: wasCreated
+          ? `Admin API added member to workspace with ${permissionLevel} permissions`
+          : `Admin API changed workspace member permissions to ${permissionLevel}`,
         metadata: { targetUserId: userId, permissions: permissionLevel },
         request,
       })
 
@@ -314,6 +314,29 @@ describe('workspace permissions route', () => {
       expect(dbChainMockFns.set).not.toHaveBeenCalled()
     })
 
+    /**
+     * The `lock_timeout` this route sets makes Postgres abort the transaction
+     * under contention. Retries cover the transient case; when they run out the
+     * caller must still get something actionable, not the driver error rendered
+     * as "Internal server error".
+     */
+    it('answers contention that outlives the retries with a busy conflict', async () => {
+      queuePersonalWorkspace([permissionRow(ADMIN_ID, 'admin'), permissionRow(MEMBER_ID, 'read')])
+      dbChainMockFns.transaction.mockRejectedValue(
+        Object.assign(new Error('canceling statement due to lock timeout'), { code: '55P03' })
+      )
+
+      const response = await PATCH(
+        createMockRequest('PATCH', { updates: [{ userId: MEMBER_ID, permissions: 'write' }] }),
+        routeContext
+      )
+
+      expect(response.status).toBe(409)
+      await expect(response.json()).resolves.toMatchObject({
+        error: 'This workspace is busy right now. Try again in a moment.',
+      })
+    })
+
     it('aborts when the workspace leaves its organization mid-request', async () => {
       queuePersonalWorkspace([permissionRow(ADMIN_ID, 'admin'), permissionRow(MEMBER_ID, 'read')])
       permissionsMockFns.mockGetWorkspaceWithOwner.mockResolvedValue({
 
@@ -3,6 +3,7 @@ import { db } from '@sim/db'
 import { member, permissions, user, workspace, workspaceEnvironment } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { isOrgAdminRole, ORG_ADMIN_ROLES } from '@sim/platform-authz/workspace'
+import { getPostgresErrorCode } from '@sim/utils/errors'
 import { and, eq, inArray, sql } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import {
@@ -14,7 +15,7 @@ import { getSession } from '@/lib/auth'
 import { HttpError } from '@/lib/core/utils/http-error'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
-import { withTransactionRetry } from '@/lib/db/transaction'
+import { isRetryableTransactionError, withTransactionRetry } from '@/lib/db/transaction'
 import type { DbOrTx } from '@/lib/db/types'
 import { captureServerEvent } from '@/lib/posthog/server'
 import {
@@ -75,11 +76,31 @@ class WorkspaceContextChangedError extends HttpError {
   }
 }
 
+/**
+ * Another writer held the rows for longer than every attempt allowed. Distinct
+ * from the conflicts above: nothing about the request is wrong and the state is
+ * unchanged, so the answer is to retry rather than to reload — and it must not
+ * reach the client as a generic server error, which is what bounding the lock
+ * wait would otherwise have produced.
+ */
+class WorkspaceBusyError extends HttpError {
+  readonly statusCode = 409
+
+  constructor() {
+    super('This workspace is busy right now. Try again in a moment.')
+    this.name = 'WorkspaceBusyError'
+  }
+}
+
 /**
  * Bounds the wait on the row locks below so a stuck holder fails fast
  * (SQLSTATE 55P03) instead of parking a pooled connection indefinitely.
+ *
+ * Kept short because `withTransactionRetry` retries that timeout: the connection
+ * is released between attempts, so three bounded waits contend better than one
+ * long one and never pin a pool slot for more than this.
  */
-const PERMISSIONS_LOCK_TIMEOUT_MS = 5000
+const PERMISSIONS_LOCK_TIMEOUT_MS = 3000
 
 /** Organization owners/admins among `userIds`, empty for a personal workspace. */
 async function loadOrgAdminTargets(
@@ -315,10 +336,11 @@ export const PATCH = withRouteHandler(
     }
 
     /**
-     * Retried on a deadlock: the ordered locks below close this route against
+     * Retried on contention: the ordered locks below close this route against
      * itself, but member removal takes the billed account before the departing
-     * user, so a cross-path cycle remains. The victim of one is asked by the
-     * database to try again, and answering it with a 500 would surface as
+     * user, so a cross-path cycle remains — and the `lock_timeout` above turns a
+     * slow competing writer into an abort of its own. Both are the database
+     * asking for a retry, and answering either with a 500 would surface as
      * "Internal server error" for a click that would have worked.
      */
     const { previousRoles, changedUserIds } = await withTransactionRetry(async (tx) => {
@@ -491,6 +513,20 @@ export const PATCH = withRouteHandler(
         previousRoles: lockedByUserId,
         changedUserIds: new Set(changedUpdates.map((update) => update.userId)),
       }
+    }).catch((error) => {
+      /**
+       * Contention that outlived every attempt. Answering with the driver error
+       * would render as "Internal server error" for a request that is simply
+       * queued behind another writer.
+       */
+      if (isRetryableTransactionError(error)) {
+        logger.warn('Permission update exhausted retries under contention', {
+          workspaceId,
+          code: getPostgresErrorCode(error),
+        })
+        throw new WorkspaceBusyError()
+      }
+      throw error
     })
 
     /**
 
@@ -0,0 +1,71 @@
+/**
+ * @vitest-environment node
+ */
+import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { isRetryableTransactionError, withTransactionRetry } from '@/lib/db/transaction'
+
+/** Shaped like the `postgres` driver's error, which carries `code` on the error. */
+function pgError(code: string): Error {
+  return Object.assign(new Error(`pg error ${code}`), { code })
+}
+
+describe('withTransactionRetry', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+  })
+
+  describe('isRetryableTransactionError', () => {
+    /**
+     * `55P03` is what a caller's own `lock_timeout` raises. Excluding it would
+     * make bounding the lock wait worse than not bounding it.
+     */
+    it.each(['40001', '40P01', '55P03'])('treats %s as retryable', (code) => {
+      expect(isRetryableTransactionError(pgError(code))).toBe(true)
+    })
+
+    it.each(['23505', '23503', '42P01'])('treats %s as terminal', (code) => {
+      expect(isRetryableTransactionError(pgError(code))).toBe(false)
+    })
+
+    it('treats a non-postgres error as terminal', () => {
+      expect(isRetryableTransactionError(new Error('boom'))).toBe(false)
+    })
+  })
+
+  it('returns the callback result without retrying when it succeeds', async () => {
+    const fn = vi.fn().mockResolvedValue('ok')
+
+    await expect(withTransactionRetry(fn)).resolves.toBe('ok')
+    expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1)
+  })
+
+  it.each(['40001', '40P01', '55P03'])(
+    'retries a %s abort and returns the later result',
+    async (code) => {
+      const fn = vi.fn().mockRejectedValueOnce(pgError(code)).mockResolvedValue('recovered')
+
+      await expect(withTransactionRetry(fn)).resolves.toBe('recovered')
+      expect(fn).toHaveBeenCalledTimes(2)
+    }
+  )
+
+  it('gives up after the attempt cap and rethrows the driver error', async () => {
+    const fn = vi.fn().mockRejectedValue(pgError('40P01'))
+
+    await expect(withTransactionRetry(fn, { attempts: 3 })).rejects.toMatchObject({ code: '40P01' })
+    expect(fn).toHaveBeenCalledTimes(3)
+  })
+
+  /**
+   * The typed domain errors callers throw to force a rollback must not be
+   * replayed — a second attempt would reach the same decision.
+   */
+  it('does not retry an error the database did not raise', async () => {
+    const fn = vi.fn().mockRejectedValue(new Error('membership changed'))
+
+    await expect(withTransactionRetry(fn)).rejects.toThrow('membership changed')
+    expect(fn).toHaveBeenCalledTimes(1)
+  })
+})
@@ -8,11 +8,16 @@ import type { DbOrTx } from '@/lib/db/types'
 const logger = createLogger('DbTransaction')
 
 /**
- * `serialization_failure` and `deadlock_detected`. Postgres raises both only
- * after aborting and fully rolling back the transaction, which is what makes a
- * fresh attempt safe rather than a duplicate.
+ * `serialization_failure`, `deadlock_detected`, and `lock_not_available`.
+ * Postgres raises all three only after aborting the transaction, so the work is
+ * fully rolled back and a fresh attempt is a retry rather than a duplicate.
+ *
+ * `55P03` is what a caller's own `lock_timeout` produces: it means a competing
+ * transaction still held the row, which is the case most likely to succeed on a
+ * second attempt. Excluding it would make bounding the wait strictly worse than
+ * not bounding it — the request would fail instead of waiting.
  */
-const RETRYABLE_TRANSACTION_CODES = new Set(['40001', '40P01'])
+const RETRYABLE_TRANSACTION_CODES = new Set(['40001', '40P01', '55P03'])
 
 const DEFAULT_ATTEMPTS = 3
 
@@ -22,15 +27,23 @@ export function isRetryableTransactionError(error: unknown): boolean {
 }
 
 /**
- * Runs `fn` in a transaction, retrying when Postgres aborts it as a deadlock
- * victim or on a serialization failure.
+ * Runs `fn` in a transaction, retrying when Postgres aborts it for contention —
+ * a deadlock, a serialization failure, or a lock timeout.
  *
- * Both are the database asking the loser to try again, and neither leaves
- * partial work behind — so without a retry they surface as a generic 500 for a
+ * All three are the database asking the loser to try again, and none leaves
+ * partial work behind, so without a retry they surface as a generic 500 for a
  * condition that would have succeeded on a second attempt. Any other error,
  * including the typed domain errors a caller throws to force a rollback,
  * propagates on the first occurrence.
  *
+ * Each attempt is its own transaction, so the pooled connection is released
+ * between them — pairing a short `lock_timeout` with a retry holds a connection
+ * for far less time than a single unbounded wait would.
+ *
+ * When the attempts are exhausted the original Postgres error propagates;
+ * callers that answer HTTP should map it with {@link isRetryableTransactionError}
+ * rather than let it become a generic 500.
+ *
  * `fn` must be free of side effects outside the transaction: it can run more
  * than once, and only the committing attempt is durable.
  */